From 7884574c7b484f0def359be16fc9c59d06f9fd4c Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 25 Apr 2013 09:15:55 +0900 Subject: [PATCH] Remove freeInboundBuffer() and freeOutboundBuffer() which has no value - Fixes #1308 freeInboundBuffer() and freeOutboundBuffer() were introduced in the early days of the new API when we did not have reference counting mechanism in the buffer. A user did not want Netty to free the handler buffers had to override these methods. However, now that we have reference counting mechanism built into the buffer, a user who wants to retain the buffers beyond handler's life cycle can simply return the buffer whose reference count is greater than 1 in newInbound/OutboundBuffer(). --- .../handler/codec/http/HttpClientCodec.java | 10 ---- .../handler/codec/http/HttpServerCodec.java | 10 ---- .../handler/codec/spdy/SpdyFrameCodec.java | 10 ---- .../handler/codec/spdy/SpdyHttpCodec.java | 10 ---- .../handler/codec/spdy/SpdyOrHttpChooser.java | 5 -- .../codec/spdy/SpdySessionHandler.java | 15 +----- .../WebSocketServerProtocolHandlerTest.java | 5 -- .../netty/handler/codec/ByteToByteCodec.java | 10 ---- .../handler/codec/ByteToMessageCodec.java | 10 ---- .../handler/codec/MessageToMessageCodec.java | 10 ---- .../handler/logging/ByteLoggingHandler.java | 10 ---- .../logging/MessageLoggingHandler.java | 10 ---- .../java/io/netty/handler/ssl/SslHandler.java | 10 ---- .../handler/stream/ChunkedWriteHandler.java | 21 ++++---- .../socket/SocketBufReleaseTest.java | 11 ---- .../io/netty/bootstrap/ServerBootstrap.java | 5 -- .../ChannelInboundByteHandlerAdapter.java | 5 -- .../netty/channel/ChannelInboundHandler.java | 10 ---- .../ChannelInboundMessageHandlerAdapter.java | 5 -- .../ChannelOutboundByteHandlerAdapter.java | 5 -- .../netty/channel/ChannelOutboundHandler.java | 10 ---- .../ChannelOutboundMessageHandlerAdapter.java | 5 -- .../channel/DefaultChannelHandlerContext.java | 45 +++++++---------- .../netty/channel/DefaultChannelPipeline.java | 12 ----- .../embedded/AbstractEmbeddedChannel.java | 14 +----- .../channel/DefaultChannelPipelineTest.java | 32 ------------ .../local/LocalTransportThreadModelTest.java | 50 ------------------- 27 files changed, 32 insertions(+), 323 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java index 36a9195af9..a91c54efa9 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java @@ -104,21 +104,11 @@ public final class HttpClientCodec decoder().discardInboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder().freeInboundBuffer(ctx); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return encoder().newOutboundBuffer(ctx); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder().freeOutboundBuffer(ctx); - } - private final class Encoder extends HttpRequestEncoder { @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java index a724f0a4d9..2018362ba4 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java @@ -67,18 +67,8 @@ public final class HttpServerCodec decoder().discardInboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder().freeInboundBuffer(ctx); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return encoder().newOutboundBuffer(ctx); } - - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder().freeOutboundBuffer(ctx); - } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameCodec.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameCodec.java index d60698cd62..67b73b1fad 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameCodec.java @@ -70,18 +70,8 @@ public final class SpdyFrameCodec decoder().discardInboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder().freeInboundBuffer(ctx); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return encoder().newOutboundBuffer(ctx); } - - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder().freeOutboundBuffer(ctx); - } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpCodec.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpCodec.java index 311e7edf8c..cfb73bf528 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpCodec.java @@ -50,18 +50,8 @@ public final class SpdyHttpCodec return decoder().newInboundBuffer(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder().freeInboundBuffer(ctx); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return encoder().newOutboundBuffer(ctx); } - - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder().freeOutboundBuffer(ctx); - } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyOrHttpChooser.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyOrHttpChooser.java index cc011d7fd1..9baca054eb 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyOrHttpChooser.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyOrHttpChooser.java @@ -72,11 +72,6 @@ public abstract class SpdyOrHttpChooser extends ChannelInboundByteHandlerAdapter // No need to discard anything because this handler will be replaced with something else very quickly. } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundByteBuffer().release(); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx, ByteBuf in) throws Exception { if (initPipeline(ctx)) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java index 0e283af67f..0062e2d3ab 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java @@ -89,21 +89,11 @@ public class SpdySessionHandler return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundMessageBuffer().release(); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundMessageBuffer().release(); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { MessageBuf in = ctx.inboundMessageBuffer(); @@ -459,7 +449,7 @@ public class SpdySessionHandler msg instanceof SpdyHeadersFrame || msg instanceof SpdyWindowUpdateFrame) { try { - handleOutboundMessage(ctx, msg, promise); + handleOutboundMessage(ctx, msg); } catch (SpdyProtocolException e) { if (e == PROTOCOL_EXCEPTION) { // on the case of PROTOCOL_EXCEPTION faile the promise directly @@ -475,8 +465,7 @@ public class SpdySessionHandler ctx.flush(promise); } - private void handleOutboundMessage(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) - throws Exception { + private void handleOutboundMessage(ChannelHandlerContext ctx, Object msg) throws Exception { if (msg instanceof SpdyDataFrame) { diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandlerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandlerTest.java index be8698b959..aeb760711a 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandlerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandlerTest.java @@ -145,11 +145,6 @@ public class WebSocketServerProtocolHandlerTest { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundMessageBuffer().release(); - } - @Override public void flush(ChannelHandlerContext ctx, ChannelPromise future) throws Exception { //NoOp diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToByteCodec.java b/codec/src/main/java/io/netty/handler/codec/ByteToByteCodec.java index f8aa592dc1..2907e6a00a 100644 --- a/codec/src/main/java/io/netty/handler/codec/ByteToByteCodec.java +++ b/codec/src/main/java/io/netty/handler/codec/ByteToByteCodec.java @@ -108,16 +108,6 @@ public abstract class ByteToByteCodec encoder.discardOutboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder.freeInboundBuffer(ctx); - } - - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder.freeOutboundBuffer(ctx); - } - /** * @see {@link ByteToByteEncoder#encode(ChannelHandlerContext, ByteBuf, ByteBuf)} */ diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java index e19b489b32..b44331b4b6 100644 --- a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java +++ b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java @@ -70,21 +70,11 @@ public abstract class ByteToMessageCodec extends ChannelDuplexHandler decoder.discardInboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder.freeInboundBuffer(ctx); - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return encoder.newOutboundBuffer(ctx); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder.freeOutboundBuffer(ctx); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { decoder.inboundBufferUpdated(ctx); diff --git a/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java b/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java index f451a2effd..f2bd26a43b 100644 --- a/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java +++ b/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java @@ -102,22 +102,12 @@ public abstract class MessageToMessageCodec return (MessageBuf) decoder.newInboundBuffer(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - decoder.freeInboundBuffer(ctx); - } - @Override @SuppressWarnings("unchecked") public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return (MessageBuf) encoder.newOutboundBuffer(ctx); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - encoder.freeOutboundBuffer(ctx); - } - @Override public void inboundBufferUpdated( ChannelHandlerContext ctx) throws Exception { diff --git a/handler/src/main/java/io/netty/handler/logging/ByteLoggingHandler.java b/handler/src/main/java/io/netty/handler/logging/ByteLoggingHandler.java index 536930f6af..dbc288ba10 100644 --- a/handler/src/main/java/io/netty/handler/logging/ByteLoggingHandler.java +++ b/handler/src/main/java/io/netty/handler/logging/ByteLoggingHandler.java @@ -117,11 +117,6 @@ public class ByteLoggingHandler ctx.inboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundByteBuffer().release(); - } - @Override public ByteBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return ChannelHandlerUtil.allocate(ctx); @@ -132,11 +127,6 @@ public class ByteLoggingHandler ctx.outboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundByteBuffer().release(); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { diff --git a/handler/src/main/java/io/netty/handler/logging/MessageLoggingHandler.java b/handler/src/main/java/io/netty/handler/logging/MessageLoggingHandler.java index 7257c0a1f9..a1a8d431e0 100644 --- a/handler/src/main/java/io/netty/handler/logging/MessageLoggingHandler.java +++ b/handler/src/main/java/io/netty/handler/logging/MessageLoggingHandler.java @@ -54,21 +54,11 @@ public class MessageLoggingHandler return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { 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 e9b45ebccd..2b0f248909 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -356,11 +356,6 @@ public class SslHandler ctx.inboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundByteBuffer().release(); - } - @Override public ByteBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return ChannelHandlerUtil.allocate(ctx); @@ -371,11 +366,6 @@ public class SslHandler ctx.outboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundByteBuffer().release(); - } - @Override public void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) throws Exception { diff --git a/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java b/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java index b318cca481..c35fa7518b 100644 --- a/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java +++ b/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java @@ -91,13 +91,20 @@ public class ChunkedWriteHandler @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - this.ctx = ctx; return queue; } @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - queue.release(); + public void handlerAdded(ChannelHandlerContext ctx) throws Exception { + this.ctx = ctx; + } + + // This method should not need any synchronization as the ChunkedWriteHandler will not receive any new events + @Override + public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { + // Fail all promised that are queued. This is needed because otherwise we would never notify the + // ChannelFuture and the registered FutureListener. See #304 + discard(ctx, new ChannelException(ChunkedWriteHandler.class.getSimpleName() + " removed from pipeline.")); } private boolean isWritable() { @@ -347,12 +354,4 @@ public class ChunkedWriteHandler } } } - - // This method should not need any synchronization as the ChunkedWriteHandler will not receive any new events - @Override - public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { - // Fail all promised that are queued. This is needed because otherwise we would never notify the - // ChannelFuture and the registered FutureListener. See #304 - discard(ctx, new ChannelException(ChunkedWriteHandler.class.getSimpleName() + " removed from pipeline.")); - } } diff --git a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java index ea22d69169..fb16050f16 100644 --- a/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java +++ b/testsuite/src/test/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java @@ -18,7 +18,6 @@ package io.netty.testsuite.transport.socket; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.buffer.ByteBuf; -import io.netty.buffer.MessageBuf; import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; @@ -79,16 +78,6 @@ public class SocketBufReleaseTest extends AbstractSocketTest { channelFuture.setSuccess(ctx.channel()); } - @Override - public MessageBuf newInboundBuffer(ChannelHandlerContext ctx) throws Exception { - return super.newInboundBuffer(ctx); - } - - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - super.freeInboundBuffer(ctx); - } - @Override public void channelActive(final ChannelHandlerContext ctx) throws Exception { byte[] data = new byte[1024]; diff --git a/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java b/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java index 144821c5c0..3049003521 100644 --- a/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java +++ b/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java @@ -267,11 +267,6 @@ public final class ServerBootstrap extends AbstractBootstrap */ Buf newInboundBuffer(ChannelHandlerContext ctx) throws Exception; - - /** - * Invoked when this handler is not going to receive any inbound message anymore and thus it's safe to - * deallocate its inbound buffer. - *

- * Please note that this method can be called from any thread repeatatively, and thus you should not perform - * stateful operation here. - *

- */ - void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception; } diff --git a/transport/src/main/java/io/netty/channel/ChannelInboundMessageHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelInboundMessageHandlerAdapter.java index b6e640402c..79a5927be6 100644 --- a/transport/src/main/java/io/netty/channel/ChannelInboundMessageHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelInboundMessageHandlerAdapter.java @@ -95,11 +95,6 @@ public abstract class ChannelInboundMessageHandlerAdapter return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundMessageBuffer().release(); - } - @Override public final void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { ChannelHandlerUtil.handleInboundBufferUpdated(ctx, this); diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundByteHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelOutboundByteHandlerAdapter.java index acbf74b44b..d5036d6e20 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundByteHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundByteHandlerAdapter.java @@ -32,11 +32,6 @@ public abstract class ChannelOutboundByteHandlerAdapter ctx.outboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundByteBuffer().release(); - } - /** * This method merely delegates the flush request to {@link #flush(ChannelHandlerContext, ByteBuf, ChannelPromise)}. */ diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java b/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java index 873b15c59a..1bcf3325b7 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundHandler.java @@ -30,14 +30,4 @@ interface ChannelOutboundHandler extends ChannelOperationHandler { *

*/ Buf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception; - - /** - * Invoked when this handler is not allowed to send any outbound message anymore and thus it's safe to - * deallocate its outbound buffer. - *

- * Please note that this method can be called from any thread repeatatively, and thus you should not perform - * stateful operation here. - *

- */ - void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception; } diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java index 02bf38aeed..4889e035a5 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java @@ -89,11 +89,6 @@ public abstract class ChannelOutboundMessageHandlerAdapter return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundMessageBuffer().release(); - } - @Override public final void flush(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception { ChannelHandlerUtil.handleFlush(ctx, promise, isCloseOnFailedFlush(), this); diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java index 665e1ea109..439d2d254c 100755 --- a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java @@ -433,28 +433,27 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap implements private void freeHandlerBuffersAfterRemoval() { int flags = this.flags; if ((flags & FLAG_REMOVED) != 0 && (flags & FLAG_FREED) == 0) { // Removed, but not freed yet - final ChannelHandler handler = handler(); try { - if (handler instanceof ChannelInboundHandler) { - try { - ((ChannelInboundHandler) handler).freeInboundBuffer(this); - } catch (Exception e) { - notifyHandlerException(e); - } - } - if (handler instanceof ChannelOutboundHandler) { - try { - ((ChannelOutboundHandler) handler).freeOutboundBuffer(this); - } catch (Exception e) { - notifyHandlerException(e); - } - } + freeBuffer(inByteBuf); + freeBuffer(inMsgBuf); + freeBuffer(outByteBuf); + freeBuffer(outMsgBuf); } finally { free(); } } } + private void freeBuffer(Buf buf) { + if (buf != null) { + try { + buf.release(); + } catch (Exception e) { + notifyHandlerException(e); + } + } + } + private void free() { flags |= FLAG_FREED; freeInbound(); @@ -1477,13 +1476,9 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap implements } private void invokeFreeInboundBuffer0() { - ChannelHandler handler = handler(); try { - if (handler instanceof ChannelInboundHandler) { - ((ChannelInboundHandler) handler).freeInboundBuffer(this); - } - } catch (Throwable t) { - notifyHandlerException(t); + freeBuffer(inByteBuf); + freeBuffer(inMsgBuf); } finally { freeInbound(); } @@ -1526,13 +1521,9 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap implements } private void invokeFreeOutboundBuffer0() { - ChannelHandler handler = handler(); try { - if (handler instanceof ChannelOutboundHandler) { - ((ChannelOutboundHandler) handler).freeOutboundBuffer(this); - } - } catch (Throwable t) { - notifyHandlerException(t); + freeBuffer(outByteBuf); + freeBuffer(outMsgBuf); } finally { freeOutbound(); } diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java index 4605e39631..ab214a256e 100755 --- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java @@ -1011,12 +1011,6 @@ final class DefaultChannelPipeline implements ChannelPipeline { throw new Error(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - byteSink.release(); - msgSink.release(); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { int byteSinkSize = byteSink.readableBytes(); @@ -1133,12 +1127,6 @@ final class DefaultChannelPipeline implements ChannelPipeline { throw new Error(); } - @Override - public final void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - msgSink.release(); - byteSink.release(); - } - @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { ctx.fireExceptionCaught(cause); diff --git a/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java b/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java index 4bb690789d..f950809652 100755 --- a/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java +++ b/transport/src/main/java/io/netty/channel/embedded/AbstractEmbeddedChannel.java @@ -52,8 +52,8 @@ public abstract class AbstractEmbeddedChannel extends AbstractChannel { private final ChannelConfig config = new DefaultChannelConfig(this); private final SocketAddress localAddress = new EmbeddedSocketAddress(); private final SocketAddress remoteAddress = new EmbeddedSocketAddress(); - private final MessageBuf lastInboundMessageBuffer = Unpooled.messageBuffer(); - private final ByteBuf lastInboundByteBuffer = Unpooled.buffer(); + private final MessageBuf lastInboundMessageBuffer = Unpooled.messageBuffer().retain(2); + private final ByteBuf lastInboundByteBuffer = Unpooled.buffer().retain(2); protected final Object lastOutboundBuffer; private Throwable lastException; private int state; // 0 = OPEN, 1 = ACTIVE, 2 = CLOSED @@ -331,11 +331,6 @@ public abstract class AbstractEmbeddedChannel extends AbstractChannel { return lastInboundMessageBuffer; } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Do NOT free the buffer. - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { // Do nothing. @@ -360,11 +355,6 @@ public abstract class AbstractEmbeddedChannel extends AbstractChannel { // nothing } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Do NOT free the buffer. - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { // No nothing diff --git a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java index 2610602b74..d2def281fc 100644 --- a/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java +++ b/transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java @@ -801,11 +801,6 @@ public class DefaultChannelPipelineTest { return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundMessageBuffer().release(); - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { updated = true; @@ -820,11 +815,6 @@ public class DefaultChannelPipelineTest { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.outboundMessageBuffer().release(); - } - @Override public void flush(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception { promise.setSuccess(); @@ -848,11 +838,6 @@ public class DefaultChannelPipelineTest { ((ChannelInboundByteHandler) stateHandler()).discardInboundReadBytes(ctx); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ((ChannelInboundHandler) stateHandler()).freeInboundBuffer(ctx); - } - @Override public ByteBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return ((ChannelOutboundByteHandler) operationHandler()).newOutboundBuffer(ctx); @@ -862,11 +847,6 @@ public class DefaultChannelPipelineTest { public void discardOutboundReadBytes(ChannelHandlerContext ctx) throws Exception { ((ChannelOutboundByteHandler) operationHandler()).discardOutboundReadBytes(ctx); } - - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ((ChannelOutboundHandler) operationHandler()).freeOutboundBuffer(ctx); - } } private static final class MessageHandlerImpl extends CombinedChannelDuplexHandler @@ -881,23 +861,11 @@ public class DefaultChannelPipelineTest { return ((ChannelInboundMessageHandler) stateHandler()).newInboundBuffer(ctx); } - @SuppressWarnings("unchecked") - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ((ChannelInboundHandler) stateHandler()).freeInboundBuffer(ctx); - } - @SuppressWarnings("unchecked") @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return ((ChannelOutboundMessageHandler) operationHandler()).newOutboundBuffer(ctx); } - - @SuppressWarnings("unchecked") - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) throws Exception { - ((ChannelOutboundHandler) operationHandler()).freeOutboundBuffer(ctx); - } } /** Test handler to validate life-cycle aware behavior. */ diff --git a/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java b/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java index f2630eb9bd..db1677f09c 100644 --- a/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java +++ b/transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java @@ -344,21 +344,11 @@ public class LocalTransportThreadModelTest { return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) { - // Nothing to free - } - @Override public void inboundBufferUpdated( ChannelHandlerContext ctx) throws Exception { @@ -406,11 +396,6 @@ public class LocalTransportThreadModelTest { return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public ByteBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return ChannelHandlerUtil.allocate(ctx); @@ -421,11 +406,6 @@ public class LocalTransportThreadModelTest { // NOOP } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) { - ctx.outboundByteBuffer().release(); - } - @Override public void inboundBufferUpdated( ChannelHandlerContext ctx) throws Exception { @@ -520,22 +500,12 @@ public class LocalTransportThreadModelTest { ctx.inboundByteBuffer().discardSomeReadBytes(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - ctx.inboundByteBuffer().release(); - } - @Override public MessageBuf newOutboundBuffer( ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) { - // Nothing to free - } - @Override public void inboundBufferUpdated( ChannelHandlerContext ctx) throws Exception { @@ -617,21 +587,11 @@ public class LocalTransportThreadModelTest { return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) { - // Nothing to free - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { Thread t = this.t; @@ -707,21 +667,11 @@ public class LocalTransportThreadModelTest { return Unpooled.messageBuffer(); } - @Override - public void freeInboundBuffer(ChannelHandlerContext ctx) throws Exception { - // Nothing to free - } - @Override public MessageBuf newOutboundBuffer(ChannelHandlerContext ctx) throws Exception { return Unpooled.messageBuffer(); } - @Override - public void freeOutboundBuffer(ChannelHandlerContext ctx) { - // Nothing to free - } - @Override public void inboundBufferUpdated(ChannelHandlerContext ctx) throws Exception { Thread t = this.t;