From d786a38139d075623ea46bb12e51e97f35081396 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 1 Feb 2011 13:39:20 +0900 Subject: [PATCH] Fixed NETTY-381 channelDisconnected event is sometimes not triggered when Channel.close() is called by multiple threads. * Internal state variable should never be set to ST_CLOSED until the close channel future is set --- .../netty/channel/local/DefaultLocalChannel.java | 13 +++++++++++-- .../netty/channel/socket/nio/NioSocketChannel.java | 12 ++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jboss/netty/channel/local/DefaultLocalChannel.java b/src/main/java/org/jboss/netty/channel/local/DefaultLocalChannel.java index 4e433942b3..bc7782ede5 100644 --- a/src/main/java/org/jboss/netty/channel/local/DefaultLocalChannel.java +++ b/src/main/java/org/jboss/netty/channel/local/DefaultLocalChannel.java @@ -27,6 +27,7 @@ import org.jboss.netty.channel.ChannelConfig; import org.jboss.netty.channel.ChannelException; import org.jboss.netty.channel.ChannelFactory; import org.jboss.netty.channel.ChannelFuture; +import org.jboss.netty.channel.ChannelFutureListener; import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.channel.ChannelSink; import org.jboss.netty.channel.DefaultChannelConfig; @@ -47,7 +48,7 @@ final class DefaultLocalChannel extends AbstractChannel implements LocalChannel private static final int ST_BOUND = 1; private static final int ST_CONNECTED = 2; private static final int ST_CLOSED = -1; - private final AtomicInteger state = new AtomicInteger(ST_OPEN); + final AtomicInteger state = new AtomicInteger(ST_OPEN); private final ChannelConfig config; private final ThreadLocalBoolean delivering = new ThreadLocalBoolean(); @@ -62,6 +63,15 @@ final class DefaultLocalChannel extends AbstractChannel implements LocalChannel super(parent, factory, pipeline, sink); this.pairedChannel = pairedChannel; config = new DefaultChannelConfig(); + + // TODO Move the state variable to AbstractChannel so that we don't need + // to add many listeners. + getCloseFuture().addListener(new ChannelFutureListener() { + public void operationComplete(ChannelFuture future) throws Exception { + state.set(ST_CLOSED); + } + }); + fireChannelOpen(this); } @@ -104,7 +114,6 @@ final class DefaultLocalChannel extends AbstractChannel implements LocalChannel @Override protected boolean setClosed() { - state.set(ST_CLOSED); return super.setClosed(); } diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioSocketChannel.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioSocketChannel.java index 3060836b62..925aa42371 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioSocketChannel.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioSocketChannel.java @@ -29,6 +29,7 @@ import org.jboss.netty.channel.AbstractChannel; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelFactory; import org.jboss.netty.channel.ChannelFuture; +import org.jboss.netty.channel.ChannelFutureListener; import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.channel.ChannelSink; import org.jboss.netty.channel.MessageEvent; @@ -50,7 +51,7 @@ class NioSocketChannel extends AbstractChannel private static final int ST_BOUND = 1; private static final int ST_CONNECTED = 2; private static final int ST_CLOSED = -1; - private volatile int state = ST_OPEN; + volatile int state = ST_OPEN; final SocketChannel socket; final NioWorker worker; @@ -82,6 +83,14 @@ class NioSocketChannel extends AbstractChannel this.socket = socket; this.worker = worker; config = new DefaultNioSocketChannelConfig(socket.socket()); + + // TODO Move the state variable to AbstractChannel so that we don't need + // to add many listeners. + getCloseFuture().addListener(new ChannelFutureListener() { + public void operationComplete(ChannelFuture future) throws Exception { + state = ST_CLOSED; + } + }); } @Override @@ -147,7 +156,6 @@ class NioSocketChannel extends AbstractChannel @Override protected boolean setClosed() { - state = ST_CLOSED; return super.setClosed(); }