Fixed a leak in a Netty ReferenceCounted resource in Gremlin Server. This is essentially a backport from 3.2.2 on master of 9913d64cd1f9050b3d047b59dcea34ee8507d2dd which fixes a problem that happens if a Frame contained a Netty Bytebuf which is ReferenceCounted and that Frame did not write downstream because of an exception (perhaps during a transactional commit seemed like the likely place that would happen) then the Bytebuf would not get cleaned up properly and Netty would issue warnings. TINKERPOP-1375 CTR
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/6be28270 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/6be28270 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/6be28270 Branch: refs/heads/TINKERPOP-1360 Commit: 6be28270598862a7a92b88a67d18e5e8f198b92b Parents: 90e5af1 Author: Stephen Mallette <sp...@genoprime.com> Authored: Mon Jul 25 12:15:30 2016 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Mon Jul 25 12:18:05 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 6 +++ .../tinkerpop/gremlin/server/handler/Frame.java | 11 +++++ .../server/op/AbstractEvalOpProcessor.java | 43 ++++++++++++-------- 3 files changed, 44 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/6be28270/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index bff4996..8735a87 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -22,6 +22,12 @@ TinkerPop 3.1.0 (A 187 On The Undercover Gremlinz) image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/images/gremlin-gangster.png[width=185] +[[release-3-1-4]] +TinkerPop 3.1.4 (NOT OFFICIALLY RELEASED YET) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* Fixed a potential leak of a `ReferenceCounted` resource in Gremlin Server. + [[release-3-1-3]] TinkerPop 3.1.3 (Release Date: July 18, 2016) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/6be28270/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/Frame.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/Frame.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/Frame.java index e6a616f..76d772e 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/Frame.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/Frame.java @@ -18,6 +18,8 @@ */ package org.apache.tinkerpop.gremlin.server.handler; +import io.netty.util.ReferenceCounted; + /** * A holder for a {@code String} or {@code ByteBuf} that represents a message to be written back to the requesting * client. @@ -34,4 +36,13 @@ public class Frame { public Object getMsg() { return msg; } + + /** + * If the object contained in the frame is {@code ReferenceCounted} then it may need to be released or else + * Netty will generate warnings that counted resources are leaking. + */ + public void tryRelease() { + if (msg instanceof ReferenceCounted) + ((ReferenceCounted) msg).release(); + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/6be28270/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java index 8f18ade..20ba1ff 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java @@ -379,31 +379,42 @@ public abstract class AbstractEvalOpProcessor implements OpProcessor { // serialize here because in sessionless requests the serialization must occur in the same // thread as the eval. as eval occurs in the GremlinExecutor there's no way to get back to the // thread that processed the eval of the script so, we have to push serialization down into that - Frame frame; + Frame frame = null; try { frame = makeFrame(ctx, msg, serializer, useBinary, aggregate, code); } catch (Exception ex) { + // a frame may use a Bytebuf which is a countable release - if it does not get written + // downstream it needs to be released here + if (frame != null) frame.tryRelease(); + // exception is handled in makeFrame() - serialization error gets written back to driver // at that point if (manageTransactions) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement); break; } - // only need to reset the aggregation list if there's more stuff to write - if (itty.hasNext()) - aggregate = new ArrayList<>(resultIterationBatchSize); - else { - // iteration and serialization are both complete which means this finished successfully. note that - // errors internal to script eval or timeout will rollback given GremlinServer's global configurations. - // local errors will get rolledback below because the exceptions aren't thrown in those cases to be - // caught by the GremlinExecutor for global rollback logic. this only needs to be committed if - // there are no more items to iterate and serialization is complete - if (managedTransactionsForRequest) attemptCommit(msg, context.getGraphManager(), settings.strictTransactionManagement); - - // exit the result iteration loop as there are no more results left. using this external control - // because of the above commit. some graphs may open a new transaction on the call to - // hasNext() - hasMore = false; + try { + // only need to reset the aggregation list if there's more stuff to write + if (itty.hasNext()) + aggregate = new ArrayList<>(resultIterationBatchSize); + else { + // iteration and serialization are both complete which means this finished successfully. note that + // errors internal to script eval or timeout will rollback given GremlinServer's global configurations. + // local errors will get rolledback below because the exceptions aren't thrown in those cases to be + // caught by the GremlinExecutor for global rollback logic. this only needs to be committed if + // there are no more items to iterate and serialization is complete + if (managedTransactionsForRequest) attemptCommit(msg, context.getGraphManager(), settings.strictTransactionManagement); + + // exit the result iteration loop as there are no more results left. using this external control + // because of the above commit. some graphs may open a new transaction on the call to + // hasNext() + hasMore = false; + } + } catch (Exception ex) { + // a frame may use a Bytebuf which is a countable release - if it does not get written + // downstream it needs to be released here + if (frame != null) frame.tryRelease(); + throw ex; } // the flush is called after the commit has potentially occurred. in this way, if a commit was
