Repository: netty/netty
Issue: NullPointerException in PendingWriteQueue
Labels: defect

Netty Version: 4.0.27.final

Hi. When there're network outages, sometimes on server side we saw handler's exceptionCaught was called with a NullPointerException in PendingWriteQueue. 
@trustin  Is it a race condition between adding SslHandler and closing channel?
Thanks.

```
java.lang.NullPointerException
        at io.netty.channel.PendingWriteQueue.recycle(PendingWriteQueue.java:248) 
        at io.netty.channel.PendingWriteQueue.removeAndWrite(PendingWriteQueue.java:196) 
        at io.netty.channel.PendingWriteQueue.removeAndWriteAll(PendingWriteQueue.java:150) 
        at io.netty.handler.ssl.SslHandler.flush(SslHandler.java:455)
        at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:735)
        at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:716)
        at io.netty.channel.ChannelOutboundHandlerAdapter.flush(ChannelOutboundHandlerAdapter.java:115)
        at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:735)
        at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:716) 
        at io.netty.channel.ChannelDuplexHandler.flush(ChannelDuplexHandler.java:117)
        at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:735)
        at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:716)
        at io.netty.channel.ChannelOutboundHandlerAdapter.flush(ChannelOutboundHandlerAdapter.java:115)
        at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:735)
        at io.netty.channel.AbstractChannelHandlerContext.access$1500(AbstractChannelHandlerContext.java:32) 
        at io.netty.channel.AbstractChannelHandlerContext$WriteAndFlushTask.write(AbstractChannelHandlerContext.java:1033)
        at io.netty.channel.AbstractChannelHandlerContext$AbstractWriteTask.run(AbstractChannelHandlerContext.java:965)
        at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:357)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:357)
        at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:111)
        at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137)
        at java.lang.Thread.run(Thread.java:745)
```


Issue comments:
That's strange... let me check

@mingyu89 thanks for reporting.. I think I know what the problem is and I'm working on a fix.

@normanmaurer Thank you!


Pull request review comments:

Patch that resolved the issue:
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) {


Test patch:
diff --git a/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java b/transport/src/test/java/io/netty/channel/PendingWriteQueueTest.java
index db2d1e8d05f..a28fbba4d62 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;