From fa5e60223872bfb55fa023146fada741a018a180 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 | 26 +++++++++++++++++++ 2 files changed, 30 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 fe19a439b6..09d88a085d 100644 --- a/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java +++ b/transport/src/test/java/io/netty/bootstrap/BootstrapTest.java @@ -161,6 +161,32 @@ public class BootstrapTest { } } + @Test + public void testLateRegisterFailed() throws Exception { + final TestEventLoopGroup group = new TestEventLoopGroup(); + 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 TestEventLoopGroup extends DefaultEventLoopGroup { ChannelPromise promise; TestEventLoopGroup() {