Repository: tinkerpop Updated Branches: refs/heads/tp32 779399a8b -> 362e77240
Used WeakReference for evaluationFuture Adjusted a previous commit - didn't get the last one right for some reason. The evaluationFuture holds on to the result and thus needs the WeakReference. In this way the result can be garbage collected once it goes out of scope and doesn't need to wait for the timeout to expire. CTR Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/362e7724 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/362e7724 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/362e7724 Branch: refs/heads/tp32 Commit: 362e77240925426574e925d665cd937507e9b698 Parents: 779399a Author: Stephen Mallette <[email protected]> Authored: Tue Jul 18 09:30:52 2017 -0400 Committer: Stephen Mallette <[email protected]> Committed: Tue Jul 18 09:30:52 2017 -0400 ---------------------------------------------------------------------- .../gremlin/groovy/engine/GremlinExecutor.java | 36 ++++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/362e7724/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java index d02d773..da495e6 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java @@ -278,7 +278,7 @@ public class GremlinExecutor implements AutoCloseable { // override the timeout if the lifecycle has a value assigned final long scriptEvalTimeOut = lifeCycle.getScriptEvaluationTimeoutOverride().orElse(scriptEvaluationTimeout); - final CompletableFuture<Object> evaluationFuture = new CompletableFuture<>(); + final WeakReference<CompletableFuture<Object>> evaluationFuture = new WeakReference<>(new CompletableFuture<>()); final FutureTask<Void> evalFuture = new FutureTask<>(() -> { try { lifeCycle.getBeforeEval().orElse(beforeEval).accept(bindings); @@ -306,39 +306,45 @@ public class GremlinExecutor implements AutoCloseable { // that must raise as an exception to the caller who has the returned evaluationFuture. in other words, // if it occurs before this point, then the handle() method won't be called again if there is an // exception that ends up below trying to completeExceptionally() - evaluationFuture.complete(result); + final CompletableFuture<Object> ef = evaluationFuture.get(); + if (ef != null) ef.complete(result); } catch (Throwable ex) { final Throwable root = null == ex.getCause() ? ex : ExceptionUtils.getRootCause(ex); // thread interruptions will typically come as the result of a timeout, so in those cases, // check for that situation and convert to TimeoutException - if (root instanceof InterruptedException) { - lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings); - evaluationFuture.completeExceptionally(new TimeoutException( - String.format("Script evaluation exceeded the configured 'scriptEvaluationTimeout' threshold of %s ms or evaluation was otherwise cancelled directly for request [%s]: %s", scriptEvalTimeOut, script, root.getMessage()))); - } else { - lifeCycle.getAfterFailure().orElse(afterFailure).accept(bindings, root); - evaluationFuture.completeExceptionally(root); + final CompletableFuture<Object> ef = evaluationFuture.get(); + if (ef != null) { + if (root instanceof InterruptedException) { + lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings); + ef.completeExceptionally(new TimeoutException( + String.format("Script evaluation exceeded the configured 'scriptEvaluationTimeout' threshold of %s ms or evaluation was otherwise cancelled directly for request [%s]: %s", scriptEvalTimeOut, script, root.getMessage()))); + } else { + lifeCycle.getAfterFailure().orElse(afterFailure).accept(bindings, root); + ef.completeExceptionally(root); + } } } return null; }); - final WeakReference<Future<?>> executionFuture = new WeakReference<>(executorService.submit(evalFuture)); + final Future<?> executionFuture = executorService.submit(evalFuture); if (scriptEvalTimeOut > 0) { // Schedule a timeout in the thread pool for future execution scheduledExecutorService.schedule(() -> { - final Future<?> f = executionFuture.get(); - if (f != null && f.cancel(true)) { + if (executionFuture.cancel(true)) { lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings); - evaluationFuture.completeExceptionally(new TimeoutException( - String.format("Script evaluation exceeded the configured 'scriptEvaluationTimeout' threshold of %s ms or evaluation was otherwise cancelled directly for request [%s]", scriptEvalTimeOut, script))); + final CompletableFuture<Object> ef = evaluationFuture.get(); + if (ef != null) { + ef.completeExceptionally(new TimeoutException( + String.format("Script evaluation exceeded the configured 'scriptEvaluationTimeout' threshold of %s ms or evaluation was otherwise cancelled directly for request [%s]", scriptEvalTimeOut, script))); + } } }, scriptEvalTimeOut, TimeUnit.MILLISECONDS); } - return evaluationFuture; + return evaluationFuture.get(); } /**
