From 2f1a0b0593af284221230d79c8d30a2d7383d4e6 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Sun, 10 Feb 2013 13:31:31 +0900 Subject: [PATCH] Remove freeInbound/OutboundMessage(), replaced by ReferenceCounted.retain/release() - Related: #1029 --- .../codec/http/HttpContentDecoder.java | 14 ++++--------- .../codec/http/HttpContentEncoder.java | 19 ++++------------- .../codec/http/HttpObjectAggregator.java | 13 ++++-------- .../WebSocketServerProtocolHandler.java | 9 ++++---- .../handler/codec/spdy/SpdyHttpEncoder.java | 11 ++-------- .../spdy/SpdyHttpResponseStreamIdHandler.java | 14 ++++--------- .../handler/codec/MessageToMessageCodec.java | 21 ------------------- .../autobahn/AutobahnServerHandler.java | 16 +++++--------- .../server/WebSocketServerHandler.java | 11 ++-------- .../sslserver/WebSocketSslServerHandler.java | 11 ++-------- .../sctp/SctpMessageCompletionHandler.java | 7 ++----- .../ChannelInboundMessageHandlerAdapter.java | 11 +--------- .../ChannelOutboundMessageHandlerAdapter.java | 11 +--------- 13 files changed, 35 insertions(+), 133 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java index c74fa91f47..c5bd0fc66e 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentDecoder.java @@ -61,7 +61,7 @@ public abstract class HttpContentDecoder extends MessageToMessageDecoder { if (!m.getDecoderResult().isSuccess()) { removeTransferEncodingChunked(m); this.currentMessage = null; + BufUtil.retain(m); return m; } if (msg instanceof HttpRequest) { @@ -156,7 +158,6 @@ public class HttpObjectAggregator extends MessageToMessageDecoder { CompositeByteBuf content = (CompositeByteBuf) currentMessage.data(); if (content.readableBytes() > maxContentLength - chunk.data().readableBytes()) { - chunk.release(); // TODO: Respond with 413 Request Entity Too Large // and discard the traffic or close the connection. // No need to notify the upstream handlers - just log. @@ -168,10 +169,9 @@ public class HttpObjectAggregator extends MessageToMessageDecoder { // Append the content of the chunk if (chunk.data().isReadable()) { + chunk.retain(); content.addComponent(chunk.data()); content.writerIndex(content.writerIndex() + chunk.data().readableBytes()); - } else { - chunk.release(); } final boolean last; @@ -180,7 +180,7 @@ public class HttpObjectAggregator extends MessageToMessageDecoder { DecoderResult.partialFailure(chunk.getDecoderResult().cause())); last = true; } else { - last = msg instanceof LastHttpContent; + last = chunk instanceof LastHttpContent; } if (last) { @@ -207,11 +207,6 @@ public class HttpObjectAggregator extends MessageToMessageDecoder { } } - @Override - protected void freeInboundMessage(HttpObject msg) throws Exception { - // decode() frees HttpContents. - } - @Override public void beforeAdd(ChannelHandlerContext ctx) throws Exception { this.ctx = ctx; diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java index 1f084d410b..4e7f8d38aa 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java @@ -78,14 +78,18 @@ public class WebSocketServerProtocolHandler extends ChannelInboundMessageHandler public void messageReceived(ChannelHandlerContext ctx, WebSocketFrame frame) throws Exception { if (frame instanceof CloseWebSocketFrame) { WebSocketServerHandshaker handshaker = getHandshaker(ctx); + frame.retain(); handshaker.close(ctx.channel(), (CloseWebSocketFrame) frame); return; } + if (frame instanceof PingWebSocketFrame) { + frame.data().retain(); ctx.channel().write(new PongWebSocketFrame(frame.data())); return; } + frame.retain(); ctx.nextInboundMessageBuffer().add(frame); } @@ -100,11 +104,6 @@ public class WebSocketServerProtocolHandler extends ChannelInboundMessageHandler } } - @Override - protected void freeInboundMessage(WebSocketFrame msg) throws Exception { - // Do not free; other handlers will. - } - static WebSocketServerHandshaker getHandshaker(ChannelHandlerContext ctx) { return ctx.attr(HANDSHAKER_ATTR_KEY).get(); } diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java index fe8fe31f07..28d7acfecd 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpEncoder.java @@ -162,6 +162,8 @@ public class SpdyHttpEncoder extends MessageToMessageEncoder { if (msg instanceof HttpContent) { HttpContent chunk = (HttpContent) msg; + + chunk.data().retain(); SpdyDataFrame spdyDataFrame = new DefaultSpdyDataFrame(currentStreamId, chunk.data()); spdyDataFrame.setLast(chunk instanceof LastHttpContent); if (chunk instanceof LastHttpContent) { @@ -284,13 +286,4 @@ public class SpdyHttpEncoder extends MessageToMessageEncoder { return spdySynReplyFrame; } - - @Override - protected void freeOutboundMessage(HttpObject msg) throws Exception { - if (msg instanceof HttpContent) { - // Will be freed later as the content of them is just reused here - return; - } - super.freeOutboundMessage(msg); - } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpResponseStreamIdHandler.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpResponseStreamIdHandler.java index 65c5871d34..1cbcbdcc4a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpResponseStreamIdHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpResponseStreamIdHandler.java @@ -15,6 +15,7 @@ */ package io.netty.handler.codec.spdy; +import io.netty.buffer.BufUtil; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.MessageToMessageCodec; import io.netty.handler.codec.http.HttpMessage; @@ -43,6 +44,8 @@ public class SpdyHttpResponseStreamIdHandler extends if (id != null && id.intValue() != NO_ID && !msg.headers().contains(SpdyHttpHeaders.Names.STREAM_ID)) { SpdyHttpHeaders.setStreamId(msg, id); } + + BufUtil.retain(msg); return msg; } @@ -59,16 +62,7 @@ public class SpdyHttpResponseStreamIdHandler extends ids.remove(((SpdyRstStreamFrame) msg).getStreamId()); } + BufUtil.retain(msg); return msg; } - - @Override - protected void freeInboundMessage(Object msg) throws Exception { - // just pass through so no free - } - - @Override - protected void freeOutboundMessage(HttpMessage msg) throws Exception { - // just pass through so no free - } } 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 bdbde8c8ae..5b3d85c730 100644 --- a/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java +++ b/codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java @@ -15,7 +15,6 @@ */ package io.netty.handler.codec; -import io.netty.buffer.BufUtil; import io.netty.buffer.MessageBuf; import io.netty.channel.ChannelDuplexHandler; import io.netty.channel.ChannelHandlerContext; @@ -68,12 +67,6 @@ public abstract class MessageToMessageCodec protected Object encode(ChannelHandlerContext ctx, Object msg) throws Exception { return MessageToMessageCodec.this.encode(ctx, (OUTBOUND_IN) msg); } - - @Override - @SuppressWarnings("unchecked") - protected void freeOutboundMessage(Object msg) throws Exception { - MessageToMessageCodec.this.freeOutboundMessage((OUTBOUND_IN) msg); - } }; private final MessageToMessageDecoder decoder = @@ -89,12 +82,6 @@ public abstract class MessageToMessageCodec protected Object decode(ChannelHandlerContext ctx, Object msg) throws Exception { return MessageToMessageCodec.this.decode(ctx, (INBOUND_IN) msg); } - - @Override - @SuppressWarnings("unchecked") - protected void freeInboundMessage(Object msg) throws Exception { - MessageToMessageCodec.this.freeInboundMessage((INBOUND_IN) msg); - } }; private final TypeParameterMatcher inboundMsgMatcher; @@ -172,12 +159,4 @@ public abstract class MessageToMessageCodec protected abstract Object encode(ChannelHandlerContext ctx, OUTBOUND_IN msg) throws Exception; protected abstract Object decode(ChannelHandlerContext ctx, INBOUND_IN msg) throws Exception; - - protected void freeInboundMessage(INBOUND_IN msg) throws Exception { - BufUtil.release(msg); - } - - protected void freeOutboundMessage(OUTBOUND_IN msg) throws Exception { - BufUtil.release(msg); - } } diff --git a/example/src/main/java/io/netty/example/http/websocketx/autobahn/AutobahnServerHandler.java b/example/src/main/java/io/netty/example/http/websocketx/autobahn/AutobahnServerHandler.java index 5d0ee8c96a..639d165dca 100644 --- a/example/src/main/java/io/netty/example/http/websocketx/autobahn/AutobahnServerHandler.java +++ b/example/src/main/java/io/netty/example/http/websocketx/autobahn/AutobahnServerHandler.java @@ -90,18 +90,22 @@ public class AutobahnServerHandler extends ChannelInboundMessageHandlerAdapter abort.expect(ABORT); break; } finally { - freeInboundMessage(imsg); + BufUtil.release(imsg); } } } catch (Throwable t) { @@ -171,13 +171,4 @@ public abstract class ChannelInboundMessageHandlerAdapter @SuppressWarnings("UnusedParameters") ChannelHandlerContext ctx) throws Exception { // NOOP } - - /** - * Is called after a message was processed via {@link #messageReceived(ChannelHandlerContext, Object)} to free - * up any resources that is held by the inbound message. You may want to override this if your implementation - * just pass-through the input message or need it for later usage. - */ - protected void freeInboundMessage(I msg) throws Exception { - BufUtil.release(msg); - } } diff --git a/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java index bdb2933e68..678593c032 100644 --- a/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelOutboundMessageHandlerAdapter.java @@ -111,7 +111,7 @@ public abstract class ChannelOutboundMessageHandlerAdapter flush(ctx, imsg); processed ++; } finally { - freeOutboundMessage(imsg); + BufUtil.release(imsg); } } } catch (Throwable t) { @@ -172,13 +172,4 @@ public abstract class ChannelOutboundMessageHandlerAdapter * @param ctx the {@link ChannelHandlerContext} which this {@link ChannelHandler} belongs to */ protected void endFlush(@SuppressWarnings("UnusedParameters") ChannelHandlerContext ctx) throws Exception { } - - /** - * Is called after a message was processed via {@link #flush(ChannelHandlerContext, Object)} to free - * up any resources that is held by the outbound message. You may want to override this if your implementation - * just pass-through the input message or need it for later usage. - */ - protected void freeOutboundMessage(I msg) throws Exception { - BufUtil.release(msg); - } }