abdullah alamoudi has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/2901
Change subject: [NO ISSUE][HTTP] Fix buffer leak in HttpServer
......................................................................
[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
---
M
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletResponse.java
M
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedNettyOutputStream.java
M
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/FullResponse.java
M
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpRequestHandler.java
4 files changed, 34 insertions(+), 1 deletion(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/01/2901/1
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/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..dfe27e2 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,16 @@
}
public void notifyChannelInactive() {
- response.notifyChannelInactive();
+ synchronized (this) {
+ if (!started) {
+ cancelled = true;
+ }
+ }
+ if (cancelled) {
+ // release request and response
+ } else {
+ response.notifyChannelInactive();
+ }
}
public void reject() throws IOException {
--
To view, visit https://asterix-gerrit.ics.uci.edu/2901
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9a34142e87158385152fa0a11be39abced307fcc
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
