Skip to content

Commit

Permalink
[#3967] Guard against NPE in PendingWriteQueue
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
normanmaurer committed Jul 17, 2015
1 parent 9e8edb3 commit ae32fd4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
14 changes: 12 additions & 2 deletions transport/src/main/java/io/netty/channel/PendingWriteQueue.java
Expand Up @@ -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);
}
}

/**
Expand Down Expand Up @@ -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) {
Expand Down
Expand Up @@ -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;
Expand Down

0 comments on commit ae32fd4

Please sign in to comment.