You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2015/02/12 20:59:22 UTC
[2/3] incubator-tinkerpop git commit: Add some AST Transformations to
inject thread interrupt and timeout checks for scripts.
Add some AST Transformations to inject thread interrupt and timeout checks for scripts.
This should help prevent errant or mischevious scripts from hurting gremlin server to a certain degree. It will not break a long run traversal or anything like that but should stop a while(true) {} or the like.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/dde07e6f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/dde07e6f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/dde07e6f
Branch: refs/heads/master
Commit: dde07e6f02eb60dce155d1dc156a86c8faaacdda
Parents: 6a7e91b
Author: Stephen Mallette <sp...@apache.org>
Authored: Thu Feb 12 14:57:28 2015 -0500
Committer: Stephen Mallette <sp...@apache.org>
Committed: Thu Feb 12 14:57:28 2015 -0500
----------------------------------------------------------------------
.../groovy/engine/GremlinExecutorTest.java | 11 +++++----
.../jsr223/GremlinGroovyScriptEngineTest.java | 14 +++++++++++
.../jsr223/GremlinGroovyScriptEngine.java | 25 ++++++++++++++++++--
3 files changed, 43 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/dde07e6f/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
index e6c06bb..a270fe7 100644
--- a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
+++ b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
@@ -29,6 +29,7 @@ import org.kohsuke.groovy.sandbox.GroovyInterceptor;
import javax.script.Bindings;
import javax.script.SimpleBindings;
+import java.sql.Time;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -37,6 +38,8 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ExecutorService;
@@ -206,7 +209,7 @@ public class GremlinExecutorTest extends AbstractGremlinTest {
// then iterated in another. note that Gremlin Server configures the script engine to auto-commit
// after evaluation. this basically tests the state of the Gremlin Server GremlinExecutor when
// being used in sessionless mode
- final ExecutorService evalExecutor = Executors.newSingleThreadExecutor(testingThreadFactory);
+ final ExecutorService evalExecutor = Executors.newFixedThreadPool(2, testingThreadFactory);
final GremlinExecutor gremlinExecutor = GremlinExecutor.build()
.afterSuccess(b -> {
final Graph graph = (Graph) b.get("g");
@@ -241,7 +244,7 @@ public class GremlinExecutorTest extends AbstractGremlinTest {
// this test sort of simulates Gremlin Server interaction where a Traversal is eval'd in one Thread, but
// then iterated in another. this basically tests the state of the Gremlin Server GremlinExecutor when
// being used in session mode
- final ExecutorService evalExecutor = Executors.newSingleThreadExecutor(testingThreadFactory);
+ final ExecutorService evalExecutor = Executors.newFixedThreadPool(2, testingThreadFactory);
final GremlinExecutor gremlinExecutor = GremlinExecutor.build().executorService(evalExecutor).create();
final Map<String,Object> bindings = new HashMap<>();
@@ -308,9 +311,7 @@ public class GremlinExecutorTest extends AbstractGremlinTest {
@Test
public void shouldNotExhaustThreads() throws Exception {
- // this is not representative of how the GremlinExecutor should be configured. A single thread executor
- // shared will create odd behaviors, but it's good for this test.
- final ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor(testingThreadFactory);
+ final ScheduledExecutorService executorService = Executors.newScheduledThreadPool(2, testingThreadFactory);
final GremlinExecutor gremlinExecutor = GremlinExecutor.build()
.executorService(executorService)
.scheduledExecutorService(executorService).create();
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/dde07e6f/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
index cd84580..5412163 100644
--- a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
+++ b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
@@ -42,6 +42,7 @@ import javax.script.CompiledScript;
import javax.script.ScriptContext;
import javax.script.ScriptEngine;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
@@ -49,6 +50,8 @@ import java.util.List;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import static org.junit.Assert.*;
@@ -564,6 +567,17 @@ public class GremlinGroovyScriptEngineTest extends AbstractGremlinTest {
assertEquals(utf8Name, eval.next());
}
+ @Test
+ public void shouldTimeoutScriptOnTimedWhile() throws Exception {
+ final ScriptEngine engine = new GremlinGroovyScriptEngine(new DefaultImportCustomizerProvider(), null, 3000);
+ try {
+ engine.eval("s = System.currentTimeMillis();\nwhile((System.currentTimeMillis() - s) < 10000) {}");
+ fail("This should have timed out");
+ } catch (ScriptException se) {
+ assertEquals(TimeoutException.class, se.getCause().getCause().getClass());
+ }
+ }
+
public static class DenyAll extends GroovyValueFilter {
@Override
public Object filter(final Object o) {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/dde07e6f/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
index 0086ff8..79d82ab 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.groovy.jsr223;
+import groovy.transform.ThreadInterrupt;
import org.apache.tinkerpop.gremlin.groovy.DefaultImportCustomizerProvider;
import org.apache.tinkerpop.gremlin.groovy.EmptyImportCustomizerProvider;
import org.apache.tinkerpop.gremlin.groovy.ImportCustomizerProvider;
@@ -35,8 +36,11 @@ import groovy.lang.MissingMethodException;
import groovy.lang.MissingPropertyException;
import groovy.lang.Script;
import groovy.lang.Tuple;
+import groovy.transform.TimedInterrupt;
+import org.codehaus.groovy.ast.tools.GeneralUtils;
import org.codehaus.groovy.control.CompilationFailedException;
import org.codehaus.groovy.control.CompilerConfiguration;
+import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer;
import org.codehaus.groovy.jsr223.GroovyCompiledScript;
import org.codehaus.groovy.jsr223.GroovyScriptEngineImpl;
import org.codehaus.groovy.runtime.InvokerHelper;
@@ -65,9 +69,9 @@ import java.util.Map;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Pattern;
-import java.util.stream.Stream;
/**
* This {@code ScriptEngine} implementation is heavily adapted from the {@code GroovyScriptEngineImpl} to include
@@ -78,6 +82,7 @@ import java.util.stream.Stream;
*/
public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl implements DependencyManager, AutoCloseable {
+ public static final long DEFAULT_SCRIPT_EVALUATION_TIMEOUT = 60000;
public static final String KEY_REFERENCE_TYPE = "#jsr223.groovy.engine.keep.globals";
public static final String REFERENCE_TYPE_PHANTOM = "phantom";
public static final String REFERENCE_TYPE_WEAK = "weak";
@@ -122,6 +127,8 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl implements
private ImportCustomizerProvider importCustomizerProvider;
private Optional<SecurityCustomizerProvider> securityProvider;
+ private final long scriptEvaluationTimeout;
+
private final Set<Artifact> artifactsToUse = new HashSet<>();
public GremlinGroovyScriptEngine() {
@@ -132,10 +139,18 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl implements
this(importCustomizerProvider, null);
}
- public GremlinGroovyScriptEngine(final ImportCustomizerProvider importCustomizerProvider, final SecurityCustomizerProvider securityCustomizerProvider) {
+ public GremlinGroovyScriptEngine(final ImportCustomizerProvider importCustomizerProvider,
+ final SecurityCustomizerProvider securityCustomizerProvider) {
+ this(importCustomizerProvider, securityCustomizerProvider, DEFAULT_SCRIPT_EVALUATION_TIMEOUT);
+ }
+
+ public GremlinGroovyScriptEngine(final ImportCustomizerProvider importCustomizerProvider,
+ final SecurityCustomizerProvider securityCustomizerProvider,
+ final long scriptEvaluationTimeout) {
GremlinLoader.load();
this.importCustomizerProvider = importCustomizerProvider;
this.securityProvider = Optional.ofNullable(securityCustomizerProvider);
+ this.scriptEvaluationTimeout = scriptEvaluationTimeout;
createClassLoader();
}
@@ -459,6 +474,12 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl implements
if (this.securityProvider.isPresent())
conf.addCompilationCustomizers(this.securityProvider.get().getCompilationCustomizer());
+ final Map<String,Object> annotationParams = new HashMap<>();
+ annotationParams.put("value", scriptEvaluationTimeout);
+ annotationParams.put("unit", GeneralUtils.propX(GeneralUtils.classX(TimeUnit.class), TimeUnit.MILLISECONDS.toString()));
+ conf.addCompilationCustomizers(new ASTTransformationCustomizer(annotationParams, TimedInterrupt.class));
+ conf.addCompilationCustomizers(new ASTTransformationCustomizer(ThreadInterrupt.class));
+
this.loader = new GremlinGroovyClassLoader(getParentLoader(), conf);
this.securityProvider.ifPresent(SecurityCustomizerProvider::registerInterceptors);
}