Repository: incubator-tinkerpop Updated Branches: refs/heads/tp31 0445e6d59 -> 2f4c24c71
The timeout function to the GremlinExecutor was not executing in the same thread as the script This would mean that if someone were to override the timeout and they were expecting to rollback a transaction or perform some other action that needed to occur in the same thread as the script they would not get the expected behavior. CTR Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/2f4c24c7 Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/2f4c24c7 Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/2f4c24c7 Branch: refs/heads/tp31 Commit: 2f4c24c71ebf4a5e455ee96ad78ca6b5e7d1306a Parents: 0445e6d Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Jun 3 20:43:02 2016 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Fri Jun 3 20:43:02 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../gremlin/groovy/engine/GremlinExecutor.java | 12 +++++------- 2 files changed, 6 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/2f4c24c7/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ebe3efd..601022a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/incubator-tinkerpop/master/docs/ TinkerPop 3.1.3 (NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Fixed a bug where timeout functions provided to the `GremlinExecutor` were not executing in the same thread as the script evaluation. * Optimized a few special cases in `RangeByIsCountStrategy`. * Named the thread pool used by Gremlin Server sessions: "gremlin-server-session-$n". * Fixed a bug in `BulkSet.equals()` which made itself apparent when using `store()` and `aggregate()` with labeled `cap()`. http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/2f4c24c7/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 5ef8667..2476114 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 @@ -249,7 +249,7 @@ public class GremlinExecutor implements AutoCloseable { bindings.putAll(boundVars); // override the timeout if the lifecycle has a value assigned - final long seto = lifeCycle.scriptEvaluationTimeoutOverride.orElse(scriptEvaluationTimeout); + final long seto = lifeCycle.getScriptEvaluationTimeoutOverride().orElse(scriptEvaluationTimeout); final CompletableFuture<Object> evaluationFuture = new CompletableFuture<>(); final FutureTask<Void> f = new FutureTask<>(() -> { @@ -283,10 +283,11 @@ public class GremlinExecutor implements AutoCloseable { // 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) + 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 for request [%s]: %s", seto, script, root.getMessage()))); - else { + } else { lifeCycle.getAfterFailure().orElse(afterFailure).accept(bindings, root); evaluationFuture.completeExceptionally(root); } @@ -301,10 +302,7 @@ public class GremlinExecutor implements AutoCloseable { // Schedule a timeout in the thread pool for future execution final ScheduledFuture<?> sf = scheduledExecutorService.schedule(() -> { logger.warn("Timing out script - {} - in thread [{}]", script, Thread.currentThread().getName()); - if (!f.isDone()) { - lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings); - f.cancel(true); - } + if (!f.isDone()) f.cancel(true); }, seto, TimeUnit.MILLISECONDS); // Cancel the scheduled timeout if the eval future is complete or the script evaluation failed
