>From Michael Blow <[email protected]>:
Michael Blow has uploaded this change for review. (
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17625 )
Change subject: [NO ISSUE][HYR][HTTP] Ensure error buffer is released on close()
......................................................................
[NO ISSUE][HYR][HTTP] Ensure error buffer is released on close()
Change-Id: I93ee1aaa8240170a4538edb9cb3c2bc975bae3be
---
M
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
1 file changed, 44 insertions(+), 17 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/25/17625/1
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 ee6d9be..5964f7b 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
@@ -45,6 +45,7 @@
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.LastHttpContent;
+import io.netty.util.ReferenceCountUtil;
/**
* A chunked http response. Here is how it is expected to work:
@@ -114,29 +115,46 @@
@Override
public void close() throws IOException {
+ Throwable ex = null;
try {
if (writer != null) {
writer.close();
}
- } finally {
- outputStream.close();
- }
- if (errorBuf == null && response.status() == HttpResponseStatus.OK) {
- if (!done) {
- respond(LastHttpContent.EMPTY_LAST_CONTENT);
- }
- } else {
- // There was an error
- if (headerSent) {
- LOGGER.log(Level.WARN, "Error after header write of chunked
response");
- if (errorBuf != null) {
- errorBuf.release();
+ if (errorBuf == null && response.status() ==
HttpResponseStatus.OK) {
+ if (!done) {
+ respond(LastHttpContent.EMPTY_LAST_CONTENT);
}
- future = ctx.channel().close().addListener(handler);
} else {
- // we didn't send anything to the user, we need to send an
non-chunked error response
- fullResponse(response.protocolVersion(), response.status(),
- errorBuf == null ? ctx.alloc().buffer(0, 0) :
errorBuf, response.headers());
+ // There was an error
+ if (headerSent) {
+ LOGGER.log(Level.WARN, "Error after header write of
chunked response");
+ future = ctx.channel().close().addListener(handler);
+ } else {
+ // we didn't send anything to the user, we need to send an
non-chunked error response
+ fullResponse(response.protocolVersion(), response.status(),
+ errorBuf == null ? ctx.alloc().buffer(0, 0) :
errorBuf, response.headers());
+ // The responsibility of releasing the error buffer is now
with the netty pipeline since it is
+ // forwarded within the http content. We must nullify
buffer to avoid releasing the buffer twice.
+ errorBuf = null;
+ }
+ }
+ } catch (Throwable t) {
+ // save any exception thrown, so that we don't suppress it in the
finally block
+ ex = t;
+ throw t;
+ } finally {
+ try {
+ outputStream.close();
+ } catch (Throwable t) {
+ if (ex != null) {
+ ex.addSuppressed(t);
+ } else {
+ throw t;
+ }
+ } finally {
+ ReferenceCountUtil.release(errorBuf);
+ // We must nullify buffer to avoid releasing the buffer twice
in case of duplicate close()
+ errorBuf = null;
}
}
done = true;
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17625
To unsubscribe, or for help writing mail filters, visit
https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: I93ee1aaa8240170a4538edb9cb3c2bc975bae3be
Gerrit-Change-Number: 17625
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Blow <[email protected]>
Gerrit-MessageType: newchange
