From c9e71aafbd58b3fed347ba5cd1d931074264f64e Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 19 Jun 2015 21:33:54 +0200 Subject: [PATCH] [#3780] Handle ChannelInitializer exception in exceptionCaught() Motivation: At the moment we directly closed the Channel when an exception accoured durring initChannel(...) without giving the user any way to do extra or special handling. Modifications: Handle the exception in exceptionCaught(...) of the ChannelInitializer which will by default log and close the Channel. This way the user can override this. Result: More felixible handling of exceptions. --- .../io/netty/channel/ChannelInitializer.java | 30 +++++++++++-------- .../channel/DefaultChannelPipelineTest.java | 24 +++++++++++++++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/ChannelInitializer.java b/transport/src/main/java/io/netty/channel/ChannelInitializer.java index f346f553cd..5db2c29180 100644 --- a/transport/src/main/java/io/netty/channel/ChannelInitializer.java +++ b/transport/src/main/java/io/netty/channel/ChannelInitializer.java @@ -56,29 +56,33 @@ public abstract class ChannelInitializer extends ChannelInbou * will be removed from the {@link ChannelPipeline} of the {@link Channel}. * * @param ch the {@link Channel} which was registered. - * @throws Exception is thrown if an error occurs. In that case the {@link Channel} will be closed. + * @throws Exception is thrown if an error occurs. In that case it will be handled by + * {@link #exceptionCaught(ChannelHandlerContext, Throwable)} which will by default close + * the {@link Channel}. */ protected abstract void initChannel(C ch) throws Exception; @Override @SuppressWarnings("unchecked") public final void channelRegistered(ChannelHandlerContext ctx) throws Exception { - ChannelPipeline pipeline = ctx.pipeline(); - boolean success = false; + initChannel((C) ctx.channel()); + ctx.pipeline().remove(this); + ctx.fireChannelRegistered(); + } + + /** + * Handle the {@link Throwable} by logging and closing the {@link Channel}. Sub-classes may override this. + */ + @Override + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { + logger.warn("Failed to initialize a channel. Closing: " + ctx.channel(), cause); try { - initChannel((C) ctx.channel()); - pipeline.remove(this); - ctx.fireChannelRegistered(); - success = true; - } catch (Throwable t) { - logger.warn("Failed to initialize a channel. Closing: " + ctx.channel(), t); - } finally { + ChannelPipeline pipeline = ctx.pipeline(); if (pipeline.context(this) != null) { pipeline.remove(this); } - if (!success) { - ctx.close(); - } + } finally { + ctx.close(); } } } diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index 40749b59db..11503bd90f 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -21,6 +21,7 @@ import io.netty.bootstrap.ServerBootstrap; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandler.Sharable; +import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.local.LocalAddress; import io.netty.channel.local.LocalChannel; import io.netty.channel.local.LocalEventLoopGroup; @@ -535,6 +536,29 @@ public class DefaultChannelPipelineTest { assertNull(pipeline.last()); } + @Test(timeout = 5000) + public void testChannelInitializerException() throws Exception { + final IllegalStateException exception = new IllegalStateException(); + final AtomicReference error = new AtomicReference(); + final CountDownLatch latch = new CountDownLatch(1); + EmbeddedChannel channel = new EmbeddedChannel(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) throws Exception { + throw exception; + } + + @Override + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { + super.exceptionCaught(ctx, cause); + error.set(cause); + latch.countDown(); + } + }); + latch.await(); + assertFalse(channel.isActive()); + assertSame(exception, error.get()); + } + private static int next(AbstractChannelHandlerContext ctx) { AbstractChannelHandlerContext next = ctx.next; if (next == null) {