From 58f4b4b7d9e6cea91a5a197d6d41f4bc2e5ff8f5 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 20 Jun 2014 20:03:22 +0200 Subject: [PATCH] [#2589] LocalServerChannel.doClose() throws NPE when localAddress == null Motivation: LocalServerChannel.doClose() calls LocalChannelRegistry.unregister(localAddress); without check if localAddress is null and so produce a NPE when pass null the used ConcurrentHashMapV8 Modification: Check for localAddress != null before try to remove it from Map. Also added a unit test which showed the stacktrace of the error. Result: No more NPE during doClose(). --- .../channel/local/LocalServerChannel.java | 6 ++-- .../io/netty/bootstrap/BootstrapTest.java | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java b/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java index 73647c8ac2..f798e56e05 100644 --- a/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java +++ b/transport/src/main/java/io/netty/channel/local/LocalServerChannel.java @@ -96,8 +96,10 @@ public class LocalServerChannel extends AbstractServerChannel { protected void doClose() throws Exception { if (state <= 1) { // Update all internal state before the closeFuture is notified. - LocalChannelRegistry.unregister(localAddress); - localAddress = null; + if (localAddress != null) { + LocalChannelRegistry.unregister(localAddress); + localAddress = null; + } state = 2; } } diff --git a/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java b/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java index 78b75ae6b3..f9e09a472c 100644 --- a/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java +++ b/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java @@ -29,12 +29,15 @@ import io.netty.channel.local.LocalChannel; import io.netty.channel.local.LocalEventLoopGroup; import io.netty.channel.local.LocalServerChannel; import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.GlobalEventExecutor; import org.junit.Assert; import org.junit.Test; import java.util.ArrayList; import java.util.List; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.LinkedBlockingQueue; public class BootstrapTest { @@ -161,6 +164,32 @@ public class BootstrapTest { } } + @Test + public void testLateRegisterFailed() throws Exception { + final TestLocalEventLoopGroup group = new TestLocalEventLoopGroup(); + try { + ServerBootstrap bootstrap = new ServerBootstrap(); + bootstrap.group(group); + bootstrap.channel(LocalServerChannel.class); + bootstrap.childHandler(new DummyHandler()); + bootstrap.localAddress(new LocalAddress("1")); + ChannelFuture future = bootstrap.bind(); + Assert.assertFalse(future.isDone()); + group.promise.setFailure(new IllegalStateException()); + final BlockingQueue queue = new LinkedBlockingQueue(); + future.addListener(new ChannelFutureListener() { + @Override + public void operationComplete(ChannelFuture future) throws Exception { + queue.add(group.next().inEventLoop(Thread.currentThread())); + } + }); + Assert.assertFalse(queue.take()); + } finally { + group.shutdownGracefully(); + group.terminationFuture().sync(); + } + } + private static final class TestLocalEventLoopGroup extends LocalEventLoopGroup { ChannelPromise promise; TestLocalEventLoopGroup() {