From e3846c54f6ff37fbf9a64e083daafe8df7155b7e Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 31 Jan 2019 20:29:17 +0100 Subject: [PATCH] =?UTF-8?q?Remove=20ChannelHandler.exceptionCaught(...)=20?= =?UTF-8?q?as=20it=20should=20only=20exist=20in=E2=80=A6=20(#8822)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: ChannelHandler.exceptionCaught(...) was marked as @deprecated as it should only exist in inbound handlers. Modifications: Remove ChannelHandler.exceptionCaught(...) and adjust code / tests. Result: Fixes https://github.com/netty/netty/issues/8527 --- .../http2/InboundHttp2ToHttpAdapterTest.java | 4 +- .../handler/codec/DatagramPacketEncoder.java | 5 --- .../EmbeddedChannelHandlerContext.java | 8 +++- .../java/io/netty/channel/ChannelHandler.java | 8 ---- .../netty/channel/ChannelHandlerAdapter.java | 12 ------ .../netty/channel/ChannelInboundHandler.java | 2 - .../channel/CombinedChannelDuplexHandler.java | 39 ++----------------- .../channel/DefaultChannelHandlerContext.java | 24 ++++++++---- .../netty/channel/DefaultChannelPipeline.java | 2 +- .../CombinedChannelDuplexHandlerTest.java | 13 +------ .../channel/DefaultChannelPipelineTest.java | 2 +- .../netty/channel/ReentrantChannelTest.java | 3 +- 12 files changed, 32 insertions(+), 90 deletions(-) diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java index 1b77fd686e..fee36fe62b 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java @@ -20,7 +20,6 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; -import io.netty.channel.ChannelHandlerAdapter; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInitializer; @@ -43,7 +42,6 @@ import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpObject; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; -import io.netty.handler.codec.http2.Http2TestUtil.Http2Runnable; import io.netty.util.AsciiString; import io.netty.util.CharsetUtil; import io.netty.util.concurrent.Future; @@ -667,7 +665,7 @@ public class InboundHttp2ToHttpAdapterTest { clientDelegator = new HttpResponseDelegator(clientListener, clientLatch, clientLatch2); p.addLast(clientDelegator); - p.addLast(new ChannelHandlerAdapter() { + p.addLast(new ChannelInboundHandlerAdapter() { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { Http2Exception e = getEmbeddedHttp2Exception(cause); diff --git a/codec/src/main/java/io/netty/handler/codec/DatagramPacketEncoder.java b/codec/src/main/java/io/netty/handler/codec/DatagramPacketEncoder.java index e8ef7ee95d..7a6f9c53ad 100644 --- a/codec/src/main/java/io/netty/handler/codec/DatagramPacketEncoder.java +++ b/codec/src/main/java/io/netty/handler/codec/DatagramPacketEncoder.java @@ -136,11 +136,6 @@ public class DatagramPacketEncoder extends MessageToMessageEncoder extends ChannelDuplexHandler { - private static final InternalLogger logger = InternalLoggerFactory.getInstance(CombinedChannelDuplexHandler.class); - private DelegatingChannelHandlerContext inboundCtx; private DelegatingChannelHandlerContext outboundCtx; private volatile boolean handlerAdded; @@ -136,35 +131,7 @@ public class CombinedChannelDuplexHandler handlerType) { - int mask = MASK_EXCEPTION_CAUGHT; + int mask = 0; try { - if (isSkippable(handlerType, "exceptionCaught", ChannelHandlerContext.class, Throwable.class)) { - mask &= ~MASK_EXCEPTION_CAUGHT; - } - if (ChannelInboundHandler.class.isAssignableFrom(handlerType)) { mask |= MASK_ALL_INBOUND; + if (isSkippable(handlerType, "exceptionCaught", ChannelHandlerContext.class, Throwable.class)) { + mask &= ~MASK_EXCEPTION_CAUGHT; + } + if (isSkippable(handlerType, "channelRegistered", ChannelHandlerContext.class)) { mask &= ~MASK_CHANNEL_REGISTERED; } @@ -375,7 +375,7 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap void invokeExceptionCaught(final Throwable cause) { try { - handler().exceptionCaught(this, cause); + ((ChannelInboundHandler) handler()).exceptionCaught(this, cause); } catch (Throwable error) { if (logger.isDebugEnabled()) { logger.debug( @@ -744,7 +744,15 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap try { ((ChannelOutboundHandler) handler()).read(this); } catch (Throwable t) { + invokeExceptionCaughtFromOutbound(t); + } + } + + private void invokeExceptionCaughtFromOutbound(Throwable t) { + if ((executionMask & MASK_EXCEPTION_CAUGHT) != 0) { notifyHandlerException(t); + } else { + findContextInbound(MASK_EXCEPTION_CAUGHT).notifyHandlerException(t); } } @@ -790,7 +798,7 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap try { ((ChannelOutboundHandler) handler()).flush(this); } catch (Throwable t) { - notifyHandlerException(t); + invokeExceptionCaughtFromOutbound(t); } } diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java index 38a2953575..56d29aac29 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java @@ -1019,7 +1019,7 @@ public class DefaultChannelPipeline implements ChannelPipeline { /** * Called once a {@link Throwable} hit the end of the {@link ChannelPipeline} without been handled by the user - * in {@link ChannelHandler#exceptionCaught(ChannelHandlerContext, Throwable)}. + * in {@link ChannelInboundHandler#exceptionCaught(ChannelHandlerContext, Throwable)}. */ protected void onUnhandledInboundException(Throwable cause) { try { diff --git a/transport/src/test/java/io/netty/channel/CombinedChannelDuplexHandlerTest.java b/transport/src/test/java/io/netty/channel/CombinedChannelDuplexHandlerTest.java index be1cd4e266..676531ce58 100644 --- a/transport/src/test/java/io/netty/channel/CombinedChannelDuplexHandlerTest.java +++ b/transport/src/test/java/io/netty/channel/CombinedChannelDuplexHandlerTest.java @@ -89,7 +89,7 @@ public class CombinedChannelDuplexHandlerTest { } @Test - public void testExceptionCaughtBothCombinedHandlers() { + public void testExceptionCaught() { final Exception exception = new Exception(); final Queue queue = new ArrayDeque<>(); @@ -101,14 +101,6 @@ public class CombinedChannelDuplexHandlerTest { ctx.fireExceptionCaught(cause); } }; - ChannelOutboundHandler outboundHandler = new ChannelOutboundHandlerAdapter() { - @Override - public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { - assertSame(exception, cause); - queue.add(this); - ctx.fireExceptionCaught(cause); - } - }; ChannelInboundHandler lastHandler = new ChannelInboundHandlerAdapter() { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { @@ -118,11 +110,10 @@ public class CombinedChannelDuplexHandlerTest { }; EmbeddedChannel channel = new EmbeddedChannel( new CombinedChannelDuplexHandler<>( - inboundHandler, outboundHandler), lastHandler); + inboundHandler, new ChannelOutboundHandlerAdapter()), lastHandler); channel.pipeline().fireExceptionCaught(exception); assertFalse(channel.finish()); assertSame(inboundHandler, queue.poll()); - assertSame(outboundHandler, queue.poll()); assertSame(lastHandler, queue.poll()); assertTrue(queue.isEmpty()); } diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index dc49e6b107..1794a3c41f 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -1488,7 +1488,7 @@ public class DefaultChannelPipelineTest { fail("handler was not one of the expected handlers"); } - private static final class CheckOrderHandler extends ChannelHandlerAdapter { + private static final class CheckOrderHandler extends ChannelInboundHandlerAdapter { private final Queue addedQueue; private final Queue removedQueue; private final AtomicReference error = new AtomicReference<>(); diff --git a/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java b/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java index 16e1f74b8f..87167e6177 100644 --- a/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java +++ b/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java @@ -258,8 +258,9 @@ public class ReentrantChannelTest extends BaseChannelTest { throw new Exception("intentional failure"); } + }, new ChannelInboundHandlerAdapter() { @Override - public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { ctx.close(); } });