diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java index 920790e365..326a8a47a9 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java @@ -67,6 +67,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { private Map childExecutors; private MessageSizeEstimator.Handle estimatorHandle; private boolean firstRegistration = true; + private boolean destroyed; /** * This is the head of a linked list that is processed by {@link #callHandlerAddedForAllHandlers()} and so process @@ -138,6 +139,12 @@ public class DefaultChannelPipeline implements ChannelPipeline { return channel; } + private void checkDestroyed(ChannelHandler handler) { + if (destroyed) { + throw new ChannelPipelineException("Channel already closed, can not add handler " + handler); + } + } + @Override public final ChannelPipeline addFirst(String name, ChannelHandler handler) { return addFirst(null, name, handler); @@ -147,6 +154,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { public final ChannelPipeline addFirst(EventExecutorGroup group, String name, ChannelHandler handler) { final AbstractChannelHandlerContext newCtx; synchronized (this) { + checkDestroyed(handler); checkMultiplicity(handler); name = filterName(name, handler); @@ -196,6 +204,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { public final ChannelPipeline addLast(EventExecutorGroup group, String name, ChannelHandler handler) { final AbstractChannelHandlerContext newCtx; synchronized (this) { + checkDestroyed(handler); checkMultiplicity(handler); newCtx = newContext(group, filterName(name, handler), handler); @@ -246,6 +255,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { final AbstractChannelHandlerContext newCtx; final AbstractChannelHandlerContext ctx; synchronized (this) { + checkDestroyed(handler); checkMultiplicity(handler); name = filterName(name, handler); ctx = getContextOrDie(baseName); @@ -306,6 +316,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { final AbstractChannelHandlerContext ctx; synchronized (this) { + checkDestroyed(handler); checkMultiplicity(handler); name = filterName(name, handler); ctx = getContextOrDie(baseName); @@ -516,6 +527,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { final AbstractChannelHandlerContext newCtx; synchronized (this) { + checkDestroyed(newHandler); checkMultiplicity(newHandler); if (newName == null) { newName = generateName(newHandler); @@ -838,6 +850,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { * See: https://github.com/netty/netty/issues/3156 */ private synchronized void destroy() { + destroyed = true; destroyUp(head.next, false); } diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index bd0727296f..a871cb4f62 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -1073,6 +1073,38 @@ public class DefaultChannelPipelineTest { group.shutdownGracefully(0, 0, TimeUnit.SECONDS); } + @Test + public void testHandlerAfterClose() throws Exception { + EmbeddedChannel channel = new EmbeddedChannel(); + channel.close().syncUninterruptibly(); + + assertFalse(channel.isActive()); + + final AtomicBoolean addedHandler = new AtomicBoolean(); + final AtomicBoolean removedHandler = new AtomicBoolean(); + + ChannelPipeline pipeline = channel.pipeline(); + try { + pipeline.addLast(new ChannelHandlerAdapter() { + @Override + public void handlerAdded(ChannelHandlerContext ctx) throws Exception { + addedHandler.set(true); + } + + @Override + public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { + removedHandler.set(true); + } + }); + fail(); + } catch (ChannelPipelineException e) { + // expected + } + + assertFalse(addedHandler.get()); + assertFalse(removedHandler.get()); + } + @Test(timeout = 3000) public void testVoidPromiseNotify() throws Throwable { ChannelPipeline pipeline1 = new LocalChannel().pipeline();