Skip to content

Commit

Permalink
Fix memory leak in http pipelining
Browse files Browse the repository at this point in the history
This is related to elastic#30801. When we calling the http pipelining
aggregator on an inbound request, we retain the netty request. However,
this is unnecessary as the pipelining aggregator does not store the
request. This worked in the past as we manually release the request and
netty internally automatically releases the request. At this point we do
not implement the ref counter interface after the pipelining step which
means that netty is no longer automatically handling this second retain.

This commit removes that retain.
  • Loading branch information
Tim-Brooks committed May 23, 2018
1 parent 28d690d commit 415c7c0
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public Netty4HttpPipeliningHandler(Logger logger, final int maxEventsHeld) {
@Override
public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
if (msg instanceof LastHttpContent) {
HttpPipelinedRequest<LastHttpContent> pipelinedRequest = aggregator.read(((LastHttpContent) msg).retain());
HttpPipelinedRequest<LastHttpContent> pipelinedRequest = aggregator.read(((LastHttpContent) msg));
ctx.fireChannelRead(pipelinedRequest);
} else {
ctx.fireChannelRead(msg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public NioHttpPipeliningHandler(Logger logger, final int maxEventsHeld) {
@Override
public void channelRead(final ChannelHandlerContext ctx, final Object msg) {
if (msg instanceof LastHttpContent) {
HttpPipelinedRequest<LastHttpContent> pipelinedRequest = aggregator.read(((LastHttpContent) msg).retain());
HttpPipelinedRequest<LastHttpContent> pipelinedRequest = aggregator.read(((LastHttpContent) msg));
ctx.fireChannelRead(pipelinedRequest);
} else {
ctx.fireChannelRead(msg);
Expand Down

0 comments on commit 415c7c0

Please sign in to comment.