diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 96e79ab34a9..da85ebbc7fd 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,6 +28,7 @@ TinkerPop 3.2.6 (Release Date: NOT OFFICIALLY RELEASED YET) This release also includes changes from <>. +* `GremlinExecutor` begins timeout of script evaluation at the time the script was submitted and not from the time it began evaluation. * `ReferenceFactory` and `DetachedFactory` now detach elements in collections accordingly. * Deprecated the `useMapperFromGraph` configuration option for Gremlin Server serializers. * `JavaTranslator` is now smart about handling `BulkSet` and `Tree`. 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 88027938b99..d646a8c2efb 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 @@ -52,9 +52,9 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.FutureTask; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -279,31 +279,6 @@ public CompletableFuture eval(final String script, final String language final CompletableFuture evaluationFuture = new CompletableFuture<>(); final FutureTask evalFuture = new FutureTask<>(() -> { - - if (scriptEvalTimeOut > 0) { - final Thread scriptEvalThread = Thread.currentThread(); - - logger.debug("Schedule timeout for script - {} - in thread [{}]", script, scriptEvalThread.getName()); - - // 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 (!evaluationFuture.isDone()) scriptEvalThread.interrupt(); - }, scriptEvalTimeOut, TimeUnit.MILLISECONDS); - - // Cancel the scheduled timeout if the eval future is complete or the script evaluation failed - // with exception - evaluationFuture.handleAsync((v, t) -> { - if (!sf.isDone()) { - logger.debug("Killing scheduled timeout on script evaluation - {} - as the eval completed (possibly with exception).", script); - sf.cancel(true); - } - - // no return is necessary - nothing downstream is concerned with what happens in here - return null; - }, scheduledExecutorService); - } - try { lifeCycle.getBeforeEval().orElse(beforeEval).accept(bindings); @@ -349,7 +324,17 @@ public CompletableFuture eval(final String script, final String language return null; }); - executorService.execute(evalFuture); + final Future executionFuture = executorService.submit(evalFuture); + if (scriptEvalTimeOut > 0) { + // Schedule a timeout in the thread pool for future execution + scheduledExecutorService.schedule(() -> { + 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))); + } + }, scriptEvalTimeOut, TimeUnit.MILLISECONDS); + } return evaluationFuture; }