From f1c5f0e0c7200f63e3e0e21a171cc19afcbed0d3 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 9 Jul 2015 22:48:10 +0200 Subject: [PATCH] [#3967] Guard against NPE in PendingWriteQueue Motivation: If the Channel is already closed when the PendingWriteQueue is created it will generate a NPE when add or remove is called later. Modifications: Add null checks to guard against NPE. Result: No more NPE possible. --- .../java/io/netty/channel/PendingWriteQueue.java | 14 ++++++++++++-- .../io/netty/channel/PendingWriteQueueTest.java | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/PendingWriteQueue.java b/transport/src/main/java/io/netty/channel/PendingWriteQueue.java index b99bef796ed..b0bb0b0f202 100644 --- a/transport/src/main/java/io/netty/channel/PendingWriteQueue.java +++ b/transport/src/main/java/io/netty/channel/PendingWriteQueue.java @@ -87,7 +87,12 @@ public void add(Object msg, ChannelPromise promise) { tail = write; } size ++; - buffer.incrementPendingOutboundBytes(write.size); + // We need to guard against null as channel.unsafe().outboundBuffer() may returned null + // if the channel was already closed when constructing the PendingWriteQueue. + // See https://github.com/netty/netty/issues/3967 + if (buffer != null) { + buffer.incrementPendingOutboundBytes(write.size); + } } /** @@ -245,7 +250,12 @@ private void recycle(PendingWrite write, boolean update) { } write.recycle(); - buffer.decrementPendingOutboundBytes(writeSize); + // We need to guard against null as channel.unsafe().outboundBuffer() may returned null + // if the channel was already closed when constructing the PendingWriteQueue. + // See https://github.com/netty/netty/issues/3967 + if (buffer != null) { + buffer.decrementPendingOutboundBytes(writeSize); + } } private static void safeFail(ChannelPromise promise, Throwable cause) { diff --git a/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java b/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java index c24254e3108..9d850a95613 100644 --- a/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java +++ b/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java @@ -244,6 +244,21 @@ public void operationComplete(ChannelFuture future) throws Exception { assertNull(channel.readInbound()); } + // See https://github.com/netty/netty/issues/3967 + @Test + public void testCloseChannelOnCreation() { + EmbeddedChannel channel = new EmbeddedChannel(new ChannelInboundHandlerAdapter()); + channel.close().syncUninterruptibly(); + + final PendingWriteQueue queue = new PendingWriteQueue(channel.pipeline().firstContext()); + + IllegalStateException ex = new IllegalStateException(); + ChannelPromise promise = channel.newPromise(); + queue.add(1L, promise); + queue.removeAndFailAll(ex); + assertSame(ex, promise.cause()); + } + private static class TestHandler extends ChannelDuplexHandler { protected PendingWriteQueue queue; private int expectedSize;