From 02ce34e894ba9f9c78ad3ee5baa99f968e362672 Mon Sep 17 00:00:00 2001 From: Slava Fomin Date: Sun, 24 Feb 2019 22:43:48 +0200 Subject: [PATCH 1/2] Fix bytebuf memory leak when a request is canceled --- .../proxy/impl/ClientToProxyConnection.java | 53 +++++++++---------- .../proxy/impl/ProxyToServerConnection.java | 23 ++++---- 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java b/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java index 3daf79102..29db1beb3 100644 --- a/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java +++ b/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java @@ -22,7 +22,7 @@ import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.timeout.IdleStateHandler; import io.netty.handler.traffic.GlobalTrafficShapingHandler; -import io.netty.util.ReferenceCounted; +import io.netty.util.ReferenceCountUtil; import io.netty.util.concurrent.EventExecutorGroup; import io.netty.util.concurrent.Future; import io.netty.util.concurrent.GenericFutureListener; @@ -355,34 +355,26 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { final HttpRequest httpRequest = (HttpRequest) msg; if (ProxyUtils.isChunked(httpRequest)) { - process(ctx, httpRequest); + process(ctx, httpRequest, true); } else { - if (httpRequest instanceof ReferenceCounted) { - LOG.debug("Retaining reference counted message"); - ((ReferenceCounted) msg).retain(); - } + ReferenceCountUtil.retain(httpRequest); proxyServer.getMessageProcessingExecutor() - .execute(wrapTask(() -> { + .execute(() -> { try { - process(ctx, httpRequest); + wrapTask(() -> process(ctx, httpRequest, false)).run(); } catch (Exception e) { ctx.fireExceptionCaught(e); } finally { - if (httpRequest instanceof ReferenceCounted) { - LOG.debug("Retaining reference counted message"); - ((ReferenceCounted) httpRequest).release(); - } + ReferenceCountUtil.release(httpRequest); } - })); + }); } } - private void process(ChannelHandlerContext ctx, HttpRequest httpRequest) { + private void process(ChannelHandlerContext ctx, HttpRequest httpRequest, boolean chunked) { - boolean authenticationRequired = false; - HttpResponse shortCircuitResponse = null; - authenticationRequired = authenticationRequired(httpRequest); + boolean authenticationRequired = authenticationRequired(httpRequest); if (authenticationRequired) { LOG.debug("Not authenticated!!"); @@ -398,9 +390,7 @@ private void process(ChannelHandlerContext ctx, HttpRequest httpRequest) { filterInstance = proxyServer.getFiltersSource().filterRequest(currentRequest, ctx); } finally { // releasing a copied http request - if (currentRequest instanceof ReferenceCounted) { - ((ReferenceCounted) currentRequest).release(); - } + ReferenceCountUtil.retain(currentRequest); } if (filterInstance != null) { currentFilters = filterInstance; @@ -409,17 +399,24 @@ private void process(ChannelHandlerContext ctx, HttpRequest httpRequest) { } // Send the request through the clientToProxyRequest filter, and respond with the short-circuit response if required - shortCircuitResponse = currentFilters.clientToProxyRequest(httpRequest); - - } + final HttpResponse shortCircuitResponse = currentFilters.clientToProxyRequest(httpRequest); - if (!authenticationRequired) { - if (httpRequest instanceof ReferenceCounted) { - LOG.debug("Retaining reference counted message"); - ((ReferenceCounted) httpRequest).retain(); + if (chunked) { + ctx.fireChannelRead(new UpstreamConnectionHandler.Request(httpRequest, shortCircuitResponse)); + } else { + ReferenceCountUtil.retain(httpRequest); + channel.eventLoop().execute(() -> { + try { + wrapTask(() -> + ctx.fireChannelRead( + new UpstreamConnectionHandler.Request(httpRequest, shortCircuitResponse)) + ).run(); + } finally { + ReferenceCountUtil.release(httpRequest); + } + }); } - ctx.fireChannelRead(new UpstreamConnectionHandler.Request(httpRequest, shortCircuitResponse)); } } } diff --git a/src/main/java/org/littleshoot/proxy/impl/ProxyToServerConnection.java b/src/main/java/org/littleshoot/proxy/impl/ProxyToServerConnection.java index 6a1c0cf02..3cb87c38c 100644 --- a/src/main/java/org/littleshoot/proxy/impl/ProxyToServerConnection.java +++ b/src/main/java/org/littleshoot/proxy/impl/ProxyToServerConnection.java @@ -32,6 +32,7 @@ import io.netty.handler.timeout.IdleStateHandler; import io.netty.handler.traffic.GlobalTrafficShapingHandler; import io.netty.util.AttributeKey; +import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCounted; import io.netty.util.concurrent.EventExecutorGroup; import io.netty.util.concurrent.Future; @@ -259,26 +260,22 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { respondWith(httpResponse); become(AWAITING_CHUNK); } else { - if (httpResponse instanceof ReferenceCounted) { - LOG.debug("Retaining reference counted message"); - ((ReferenceCounted) httpResponse).retain(); - } + ReferenceCountUtil.retain(httpResponse); proxyServer.getMessageProcessingExecutor() - .execute(clientConnection.wrapTask(() -> { + .execute(() -> { try { - respondWith(httpResponse); - currentFilters.serverToProxyResponseReceived(); - become(AWAITING_INITIAL); + clientConnection.wrapTask(() -> { + respondWith(httpResponse); + currentFilters.serverToProxyResponseReceived(); + become(AWAITING_INITIAL); + }).run(); } catch (Exception e) { exceptionCaught(ctx, e); } finally { - if (httpResponse instanceof ReferenceCounted) { - LOG.debug("Retaining reference counted message"); - ((ReferenceCounted) httpResponse).release(); - } + ReferenceCountUtil.release(httpResponse); } - })); + }); } } From aa02f4785a04dfc2f41b0c590e44a1ca00b45ac6 Mon Sep 17 00:00:00 2001 From: Slava Fomin Date: Sun, 24 Feb 2019 23:07:44 +0200 Subject: [PATCH 2/2] Fix tests --- .../proxy/impl/ClientToProxyConnection.java | 2 +- .../proxy/impl/UpstreamConnectionHandler.java | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java b/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java index 29db1beb3..f7ff1ebd9 100644 --- a/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java +++ b/src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java @@ -390,7 +390,7 @@ private void process(ChannelHandlerContext ctx, HttpRequest httpRequest, boolean filterInstance = proxyServer.getFiltersSource().filterRequest(currentRequest, ctx); } finally { // releasing a copied http request - ReferenceCountUtil.retain(currentRequest); + ReferenceCountUtil.release(currentRequest); } if (filterInstance != null) { currentFilters = filterInstance; diff --git a/src/main/java/org/littleshoot/proxy/impl/UpstreamConnectionHandler.java b/src/main/java/org/littleshoot/proxy/impl/UpstreamConnectionHandler.java index 612384d95..196256f0c 100644 --- a/src/main/java/org/littleshoot/proxy/impl/UpstreamConnectionHandler.java +++ b/src/main/java/org/littleshoot/proxy/impl/UpstreamConnectionHandler.java @@ -4,7 +4,6 @@ import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; -import io.netty.util.ReferenceCountUtil; public class UpstreamConnectionHandler extends ChannelInboundHandlerAdapter { @@ -16,14 +15,10 @@ public class UpstreamConnectionHandler extends ChannelInboundHandlerAdapter { @Override public void channelRead(ChannelHandlerContext ctx, Object request) { - try { - final ConnectionState connectionState = - clientToProxyConnection.setupUpstreamConnection(((Request) request).getShortCircuitResponse(), - ((Request) request).getInitialRequest()); - clientToProxyConnection.become(connectionState); - } finally { - ReferenceCountUtil.release(((Request) request).getInitialRequest()); - } + final ConnectionState connectionState = + clientToProxyConnection.setupUpstreamConnection(((Request) request).getShortCircuitResponse(), + ((Request) request).getInitialRequest()); + clientToProxyConnection.become(connectionState); } @Override