[NO ISSUE][HTTP] Fix buffer leak in HttpServer
- user model changes: no
- storage format changes: no
- interface changes: yes
Details:
- Prior to this change, cancelled requests before
they start leak request and response buffers.
- After this change, we distinguish between cancellation
of requests before they start or after and release resources
accordingly.
Change-Id: I9a34142e87158385152fa0a11be39abced307fcc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2901
Reviewed-by: Michael Blow <mblow@apache.org>
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletResponse.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletResponse.java
index 38f2d23..95f8f27 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletResponse.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletResponse.java
@@ -86,4 +86,9 @@
* Notifies the response that the channel has become inactive.
*/
void notifyChannelInactive();
+
+ /**
+ * Called on a created request that is cancelled before it is started
+ */
+ void cancel();
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java
index 891cc2a..adea133 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java
@@ -29,6 +29,7 @@
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http.DefaultHttpContent;
import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.util.ReferenceCountUtil;
import io.netty.util.internal.OutOfDirectMemoryError;
public class ChunkedNettyOutputStream extends OutputStream {
@@ -137,4 +138,8 @@
public synchronized void channelWritabilityChanged() {
notifyAll();
}
+
+ public void cancel() {
+ ReferenceCountUtil.release(buffer);
+ }
}
\ No newline at end of file
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
index cd746b1..f02654e 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
@@ -192,4 +192,9 @@
public void notifyChannelInactive() {
outputStream.channelWritabilityChanged();
}
+
+ @Override
+ public void cancel() {
+ outputStream.cancel();
+ }
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/FullResponse.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/FullResponse.java
index 90e33b6..1d28472 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/FullResponse.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/FullResponse.java
@@ -111,4 +111,9 @@
// Do nothing.
// This response is sent as a single piece
}
+
+ @Override
+ public void cancel() {
+ // Do nothing, as this response doesn't allocate buffers in constructor
+ }
}
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpRequestHandler.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpRequestHandler.java
index 72a8ea0..1c0801c 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpRequestHandler.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpRequestHandler.java
@@ -40,6 +40,8 @@
private final IServlet servlet;
private final IServletRequest request;
private final IServletResponse response;
+ private boolean started = false;
+ private boolean cancelled = false;
public HttpRequestHandler(ChannelHandlerContext ctx, IServlet servlet, IServletRequest request, int chunkSize) {
this.ctx = ctx;
@@ -52,6 +54,13 @@
@Override
public Void call() throws Exception {
+ synchronized (this) {
+ if (cancelled) {
+ LOGGER.warn("Request cancelled before it is started");
+ return null;
+ }
+ started = true;
+ }
try {
ChannelFuture lastContentFuture = handle();
if (!HttpUtil.isKeepAlive(request.getHttpRequest())) {
@@ -83,7 +92,18 @@
}
public void notifyChannelInactive() {
- response.notifyChannelInactive();
+ synchronized (this) {
+ if (!started) {
+ cancelled = true;
+ }
+ }
+ if (cancelled) {
+ // release request and response
+ response.cancel();
+ request.getHttpRequest().release();
+ } else {
+ response.notifyChannelInactive();
+ }
}
public void reject() throws IOException {