From 70a51bcd8d9044bdf3b73e3da7aa29482f61e05d Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 14 Mar 2013 15:51:33 +0900 Subject: [PATCH] Fix memory leak in AbstractEmbeddedChannel - Allow a transport implementation to perform an arbitrary task when a channel has been deregistered completely --- .../main/java/io/netty/channel/AbstractChannel.java | 12 +++++++++--- .../channel/embedded/AbstractEmbeddedChannel.java | 9 +++++++-- .../java/io/netty/channel/local/LocalChannel.java | 5 +++-- .../io/netty/channel/local/LocalServerChannel.java | 5 +++-- .../io/netty/channel/nio/AbstractNioChannel.java | 3 ++- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/AbstractChannel.java b/transport/src/main/java/io/netty/channel/AbstractChannel.java index a827c9bb18..338edb6ac3 100644 --- a/transport/src/main/java/io/netty/channel/AbstractChannel.java +++ b/transport/src/main/java/io/netty/channel/AbstractChannel.java @@ -741,8 +741,9 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha return; } + Runnable postTask = null; try { - doDeregister(); + postTask = doDeregister(); } catch (Throwable t) { logger.warn("Unexpected exception occurred while deregistering a channel.", t); } finally { @@ -756,6 +757,10 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha // close() calls deregister() again - no need to fire channelUnregistered. promise.setSuccess(); } + + if (postTask != null) { + postTask.run(); + } } } else { eventLoop().execute(new Runnable() { @@ -973,11 +978,12 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha /** * Deregister the {@link Channel} from its {@link EventLoop}. + * You can return a {@link Runnable} which will be run as post-task of the registration process. * * Sub-classes may override this method */ - protected void doDeregister() throws Exception { - // NOOP + protected Runnable doDeregister() throws Exception { + return null; } /** diff --git a/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java b/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java index a132a3bd81..c9cc5cc45c 100755 --- a/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java +++ b/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java @@ -225,8 +225,13 @@ public abstract class AbstractEmbeddedChannel extends AbstractChannel { } @Override - protected void doDeregister() throws Exception { - // NOOP + protected Runnable doDeregister() throws Exception { + return new Runnable() { + @Override + public void run() { + runPendingTasks(); + } + }; } @Override diff --git a/transport/src/main/java/io/netty/channel/local/LocalChannel.java b/transport/src/main/java/io/netty/channel/local/LocalChannel.java index f025a4d7a6..734d4c5396 100755 --- a/transport/src/main/java/io/netty/channel/local/LocalChannel.java +++ b/transport/src/main/java/io/netty/channel/local/LocalChannel.java @@ -26,8 +26,8 @@ import io.netty.channel.ChannelPipeline; import io.netty.channel.ChannelPromise; import io.netty.channel.DefaultChannelConfig; import io.netty.channel.EventLoop; -import io.netty.util.concurrent.SingleThreadEventExecutor; import io.netty.channel.SingleThreadEventLoop; +import io.netty.util.concurrent.SingleThreadEventExecutor; import java.net.SocketAddress; import java.nio.channels.AlreadyConnectedException; @@ -201,11 +201,12 @@ public class LocalChannel extends AbstractChannel { } @Override - protected void doDeregister() throws Exception { + protected Runnable doDeregister() throws Exception { if (isOpen()) { unsafe().close(unsafe().voidFuture()); } ((SingleThreadEventExecutor) eventLoop()).removeShutdownHook(shutdownHook); + return null; } @Override diff --git a/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java b/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java index 2b68be9873..2ff36af2d9 100755 --- a/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java +++ b/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java @@ -22,8 +22,8 @@ import io.netty.channel.ChannelPipeline; import io.netty.channel.DefaultChannelConfig; import io.netty.channel.EventLoop; import io.netty.channel.ServerChannel; -import io.netty.util.concurrent.SingleThreadEventExecutor; import io.netty.channel.SingleThreadEventLoop; +import io.netty.util.concurrent.SingleThreadEventExecutor; import java.net.SocketAddress; @@ -126,8 +126,9 @@ public class LocalServerChannel extends AbstractServerChannel { } @Override - protected void doDeregister() throws Exception { + protected Runnable doDeregister() throws Exception { ((SingleThreadEventExecutor) eventLoop()).removeShutdownHook(shutdownHook); + return null; } @Override diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java index 5a671a0e33..5ea9a29360 100755 --- a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java +++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java @@ -273,8 +273,9 @@ public abstract class AbstractNioChannel extends AbstractChannel { } @Override - protected void doDeregister() throws Exception { + protected Runnable doDeregister() throws Exception { eventLoop().cancel(selectionKey()); + return null; } @Override