diff --git a/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java index 8976eb8a89..55e596f7f6 100644 --- a/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java +++ b/transport/src/main/java/io/netty/channel/AbstractChannelHandlerContext.java @@ -475,7 +475,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap if (localAddress == null) { throw new NullPointerException("localAddress"); } - if (!validatePromise(promise, false)) { + if (isNotValidPromise(promise, false)) { // cancelled return promise; } @@ -519,7 +519,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap if (remoteAddress == null) { throw new NullPointerException("remoteAddress"); } - if (!validatePromise(promise, false)) { + if (isNotValidPromise(promise, false)) { // cancelled return promise; } @@ -553,7 +553,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap @Override public ChannelFuture disconnect(final ChannelPromise promise) { - if (!validatePromise(promise, false)) { + if (isNotValidPromise(promise, false)) { // cancelled return promise; } @@ -597,7 +597,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap @Override public ChannelFuture close(final ChannelPromise promise) { - if (!validatePromise(promise, false)) { + if (isNotValidPromise(promise, false)) { // cancelled return promise; } @@ -632,7 +632,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap @Override public ChannelFuture deregister(final ChannelPromise promise) { - if (!validatePromise(promise, false)) { + if (isNotValidPromise(promise, false)) { // cancelled return promise; } @@ -711,7 +711,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap } try { - if (!validatePromise(promise, true)) { + if (isNotValidPromise(promise, true)) { ReferenceCountUtil.release(msg); // cancelled return promise; @@ -785,7 +785,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap throw new NullPointerException("msg"); } - if (!validatePromise(promise, true)) { + if (isNotValidPromise(promise, true)) { ReferenceCountUtil.release(msg); // cancelled return promise; @@ -894,7 +894,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap return new FailedChannelFuture(channel(), executor(), cause); } - private boolean validatePromise(ChannelPromise promise, boolean allowVoidPromise) { + private boolean isNotValidPromise(ChannelPromise promise, boolean allowVoidPromise) { if (promise == null) { throw new NullPointerException("promise"); } @@ -905,7 +905,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap // // See https://github.com/netty/netty/issues/2349 if (promise.isCancelled()) { - return false; + return true; } throw new IllegalArgumentException("promise already done: " + promise); } @@ -916,7 +916,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap } if (promise.getClass() == DefaultChannelPromise.class) { - return true; + return false; } if (!allowVoidPromise && promise instanceof VoidChannelPromise) { @@ -928,7 +928,7 @@ abstract class AbstractChannelHandlerContext extends DefaultAttributeMap throw new IllegalArgumentException( StringUtil.simpleClassName(AbstractChannel.CloseFuture.class) + " not allowed in a pipeline"); } - return true; + return false; } private AbstractChannelHandlerContext findContextInbound() { diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index 83eefbc783..4f745e439c 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -504,6 +504,49 @@ public class DefaultChannelPipelineTest { assertTrue(future.isCancelled()); } + @Test(expected = IllegalArgumentException.class) + public void testWrongPromiseChannel() throws Exception { + ChannelPipeline pipeline = new LocalChannel().pipeline(); + group.register(pipeline.channel()).sync(); + + ChannelPipeline pipeline2 = new LocalChannel().pipeline(); + group.register(pipeline2.channel()).sync(); + + try { + ChannelPromise promise2 = pipeline2.channel().newPromise(); + pipeline.close(promise2); + } finally { + pipeline.close(); + pipeline2.close(); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testUnexpectedVoidChannelPromise() throws Exception { + ChannelPipeline pipeline = new LocalChannel().pipeline(); + group.register(pipeline.channel()).sync(); + + try { + ChannelPromise promise = new VoidChannelPromise(pipeline.channel(), false); + pipeline.close(promise); + } finally { + pipeline.close(); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testUnexpectedVoidChannelPromiseCloseFuture() throws Exception { + ChannelPipeline pipeline = new LocalChannel().pipeline(); + group.register(pipeline.channel()).sync(); + + try { + ChannelPromise promise = (ChannelPromise) pipeline.channel().closeFuture(); + pipeline.close(promise); + } finally { + pipeline.close(); + } + } + @Test public void testCancelDeregister() throws Exception { ChannelPipeline pipeline = new LocalChannel().pipeline();