From bdab831ba5deae6871bcb571d91207848b762744 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Fri, 25 Apr 2014 16:40:42 +0900 Subject: [PATCH] Undeprecate deregister() and chanelUnregistered() Motivation: As discussed in #2250, it will become much less complicated to implement deregistration and reregistration of a channel once #2250 is resolved. Therefore, there's no need to deprecate deregister() and channelUnregistered(). Modification: - Undeprecate deregister() and channelUnregistered() - Remove SuppressWarnings annotations where applicable Result: We (including @jakobbuchgraber) are now ready to play with #2250 at master --- .../java/io/netty/handler/ssl/SslHandler.java | 1 - .../main/java/io/netty/channel/Channel.java | 3 - .../java/io/netty/channel/ChannelHandler.java | 5 - .../netty/channel/ChannelHandlerAdapter.java | 4 +- .../netty/channel/ChannelHandlerContext.java | 3 - .../netty/channel/ChannelInboundHandler.java | 3 - .../netty/channel/ChannelOutboundHandler.java | 1 - .../io/netty/channel/ChannelPipeline.java | 3 - .../io/netty/channel/group/ChannelGroup.java | 2 - .../channel/group/DefaultChannelGroup.java | 2 - .../netty/channel/ReentrantChannelTest.java | 139 +++++++++++++++++- 11 files changed, 138 insertions(+), 28 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index b38725ecee..68d1e94109 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -370,7 +370,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH } @Override - @Deprecated public void deregister(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception { ctx.deregister(promise); } diff --git a/transport/src/main/java/io/netty/channel/Channel.java b/transport/src/main/java/io/netty/channel/Channel.java index 67ad409436..536dfac4aa 100644 --- a/transport/src/main/java/io/netty/channel/Channel.java +++ b/transport/src/main/java/io/netty/channel/Channel.java @@ -287,7 +287,6 @@ public interface Channel extends AttributeMap, Comparable { * {@link Channel}. * */ - @Deprecated ChannelFuture deregister(); /** @@ -375,7 +374,6 @@ public interface Channel extends AttributeMap, Comparable { * method called of the next {@link ChannelOutboundHandler} contained in the {@link ChannelPipeline} of the * {@link Channel}. */ - @Deprecated ChannelFuture deregister(ChannelPromise promise); /** @@ -489,7 +487,6 @@ public interface Channel extends AttributeMap, Comparable { * Deregister the {@link Channel} of the {@link ChannelPromise} from {@link EventLoop} and notify the * {@link ChannelPromise} once the operation was complete. */ - @Deprecated void deregister(ChannelPromise promise); /** diff --git a/transport/src/main/java/io/netty/channel/ChannelHandler.java b/transport/src/main/java/io/netty/channel/ChannelHandler.java index 4c3cd35b2b..b1e978e2d3 100644 --- a/transport/src/main/java/io/netty/channel/ChannelHandler.java +++ b/transport/src/main/java/io/netty/channel/ChannelHandler.java @@ -189,12 +189,7 @@ public interface ChannelHandler { /** * Gets called if a {@link Throwable} was thrown. - * - * @deprecated Will be removed in the future and only {@link ChannelInboundHandler} will receive - * exceptionCaught events. For {@link ChannelOutboundHandler} the {@link ChannelPromise} - * must be failed. */ - @Deprecated void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception; /** diff --git a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java index 3cb1cc29f5..1f9496562e 100644 --- a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java @@ -81,10 +81,8 @@ public abstract class ChannelHandlerAdapter implements ChannelHandler { * * Sub-classes may override this method to change behavior. */ - @Deprecated @Override - public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) - throws Exception { + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { ctx.fireExceptionCaught(cause); } } diff --git a/transport/src/main/java/io/netty/channel/ChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/ChannelHandlerContext.java index 0fca9d71f0..2d03c6babd 100644 --- a/transport/src/main/java/io/netty/channel/ChannelHandlerContext.java +++ b/transport/src/main/java/io/netty/channel/ChannelHandlerContext.java @@ -177,7 +177,6 @@ public interface ChannelHandlerContext * called of the next {@link ChannelInboundHandler} contained in the {@link ChannelPipeline} of the * {@link Channel}. */ - @Deprecated ChannelHandlerContext fireChannelUnregistered(); /** @@ -311,7 +310,6 @@ public interface ChannelHandlerContext * {@link Channel}. * */ - @Deprecated ChannelFuture deregister(); /** @@ -399,7 +397,6 @@ public interface ChannelHandlerContext * method called of the next {@link ChannelOutboundHandler} contained in the {@link ChannelPipeline} of the * {@link Channel}. */ - @Deprecated ChannelFuture deregister(ChannelPromise promise); /** diff --git a/transport/src/main/java/io/netty/channel/ChannelInboundHandler.java b/transport/src/main/java/io/netty/channel/ChannelInboundHandler.java index dc5a428b98..56693593f5 100644 --- a/transport/src/main/java/io/netty/channel/ChannelInboundHandler.java +++ b/transport/src/main/java/io/netty/channel/ChannelInboundHandler.java @@ -28,10 +28,7 @@ public interface ChannelInboundHandler extends ChannelHandler { /** * The {@link Channel} of the {@link ChannelHandlerContext} was unregistered from its {@link EventLoop} - * - * @deprecated use {@link #channelInactive(ChannelHandlerContext)} */ - @Deprecated void channelUnregistered(ChannelHandlerContext ctx) throws Exception; /** diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java b/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java index 0795198195..92a3943ad0 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java @@ -69,7 +69,6 @@ public interface ChannelOutboundHandler extends ChannelHandler { * @param promise the {@link ChannelPromise} to notify once the operation completes * @throws Exception thrown if an error accour */ - @Deprecated void deregister(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception; /** diff --git a/transport/src/main/java/io/netty/channel/ChannelPipeline.java b/transport/src/main/java/io/netty/channel/ChannelPipeline.java index 2ac3e52c48..4270b06e79 100644 --- a/transport/src/main/java/io/netty/channel/ChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/ChannelPipeline.java @@ -612,7 +612,6 @@ public interface ChannelPipeline * called of the next {@link ChannelInboundHandler} contained in the {@link ChannelPipeline} of the * {@link Channel}. */ - @Deprecated ChannelPipeline fireChannelUnregistered(); /** @@ -746,7 +745,6 @@ public interface ChannelPipeline * {@link Channel}. * */ - @Deprecated ChannelFuture deregister(); /** @@ -834,7 +832,6 @@ public interface ChannelPipeline * method called of the next {@link ChannelOutboundHandler} contained in the {@link ChannelPipeline} of the * {@link Channel}. */ - @Deprecated ChannelFuture deregister(ChannelPromise promise); /** diff --git a/transport/src/main/java/io/netty/channel/group/ChannelGroup.java b/transport/src/main/java/io/netty/channel/group/ChannelGroup.java index 85cfa6e94b..7c0caf27d0 100644 --- a/transport/src/main/java/io/netty/channel/group/ChannelGroup.java +++ b/transport/src/main/java/io/netty/channel/group/ChannelGroup.java @@ -220,7 +220,6 @@ public interface ChannelGroup extends Set, Comparable { * @return the {@link ChannelGroupFuture} instance that notifies when * the operation is done for all channels */ - @Deprecated ChannelGroupFuture deregister(); /** @@ -232,6 +231,5 @@ public interface ChannelGroup extends Set, Comparable { * @return the {@link ChannelGroupFuture} instance that notifies when * the operation is done for all channels */ - @Deprecated ChannelGroupFuture deregister(ChannelMatcher matcher); } diff --git a/transport/src/main/java/io/netty/channel/group/DefaultChannelGroup.java b/transport/src/main/java/io/netty/channel/group/DefaultChannelGroup.java index c5bb5be008..041be4df4c 100644 --- a/transport/src/main/java/io/netty/channel/group/DefaultChannelGroup.java +++ b/transport/src/main/java/io/netty/channel/group/DefaultChannelGroup.java @@ -173,7 +173,6 @@ public class DefaultChannelGroup extends AbstractSet implements Channel } @Override - @Deprecated public ChannelGroupFuture deregister() { return deregister(ChannelMatchers.all()); } @@ -277,7 +276,6 @@ public class DefaultChannelGroup extends AbstractSet implements Channel } @Override - @Deprecated public ChannelGroupFuture deregister(ChannelMatcher matcher) { if (matcher == null) { throw new NullPointerException("matcher"); diff --git a/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java b/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java index 3f75017e3d..b6fc1e6541 100644 --- a/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java +++ b/transport/src/test/java/io/netty/channel/ReentrantChannelTest.java @@ -15,14 +15,18 @@ */ package io.netty.channel; -import static org.junit.Assert.assertTrue; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.channel.LoggingHandler.Event; import io.netty.channel.local.LocalAddress; - +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.GenericFutureListener; import org.junit.Test; +import java.nio.channels.ClosedChannelException; + +import static org.junit.Assert.*; + public class ReentrantChannelTest extends BaseChannelTest { @Test @@ -96,4 +100,135 @@ public class ReentrantChannelTest extends BaseChannelTest { "WRITABILITY: writable=true\n"); } + @Test + public void testWriteFlushPingPong() throws Exception { + + LocalAddress addr = new LocalAddress("testWriteFlushPingPong"); + + ServerBootstrap sb = getLocalServerBootstrap(); + sb.bind(addr).sync().channel(); + + Bootstrap cb = getLocalClientBootstrap(); + + setInterest(Event.WRITE, Event.FLUSH, Event.CLOSE, Event.EXCEPTION); + + Channel clientChannel = cb.connect(addr).sync().channel(); + + clientChannel.pipeline().addLast(new ChannelOutboundHandlerAdapter() { + + int writeCount; + int flushCount; + + @Override + public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { + if (writeCount < 5) { + writeCount++; + ctx.channel().flush(); + } + super.write(ctx, msg, promise); + } + + @Override + public void flush(ChannelHandlerContext ctx) throws Exception { + if (flushCount < 5) { + flushCount++; + ctx.channel().write(createTestBuf(2000)); + } + super.flush(ctx); + } + }); + + clientChannel.writeAndFlush(createTestBuf(2000)); + clientChannel.close().sync(); + + assertLog( + "WRITE\n" + + "FLUSH\n" + + "WRITE\n" + + "FLUSH\n" + + "WRITE\n" + + "FLUSH\n" + + "WRITE\n" + + "FLUSH\n" + + "WRITE\n" + + "FLUSH\n" + + "WRITE\n" + + "FLUSH\n" + + "CLOSE\n"); + } + + @Test + public void testCloseInFlush() throws Exception { + + LocalAddress addr = new LocalAddress("testCloseInFlush"); + + ServerBootstrap sb = getLocalServerBootstrap(); + sb.bind(addr).sync().channel(); + + Bootstrap cb = getLocalClientBootstrap(); + + setInterest(Event.WRITE, Event.FLUSH, Event.CLOSE, Event.EXCEPTION); + + Channel clientChannel = cb.connect(addr).sync().channel(); + + clientChannel.pipeline().addLast(new ChannelOutboundHandlerAdapter() { + + @Override + public void write(final ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { + promise.addListener(new GenericFutureListener>() { + @Override + public void operationComplete(Future future) throws Exception { + ctx.channel().close(); + } + }); + super.write(ctx, msg, promise); + ctx.channel().flush(); + } + }); + + clientChannel.write(createTestBuf(2000)).sync(); + clientChannel.closeFuture().sync(); + + assertLog("WRITE\nFLUSH\nCLOSE\n"); + } + + @Test + public void testFlushFailure() throws Exception { + + LocalAddress addr = new LocalAddress("testFlushFailure"); + + ServerBootstrap sb = getLocalServerBootstrap(); + sb.bind(addr).sync().channel(); + + Bootstrap cb = getLocalClientBootstrap(); + + setInterest(Event.WRITE, Event.FLUSH, Event.CLOSE, Event.EXCEPTION); + + Channel clientChannel = cb.connect(addr).sync().channel(); + + clientChannel.pipeline().addLast(new ChannelOutboundHandlerAdapter() { + + @Override + public void flush(ChannelHandlerContext ctx) throws Exception { + throw new Exception("intentional failure"); + } + + @Override + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { + ctx.close(); + } + }); + + try { + clientChannel.writeAndFlush(createTestBuf(2000)).sync(); + fail(); + } catch (Throwable cce) { + // FIXME: shouldn't this contain the "intentional failure" exception? + assertEquals(ClosedChannelException.class, cce.getClass()); + } + + clientChannel.closeFuture().sync(); + + assertLog("WRITE\nCLOSE\n"); + } }