8023560: Arbitrary javax.script.Bindings objects as ENGINE_SCOPE objects are not handled as expected.
Reviewed-by: jlaskey, lagergren, hannesw
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Thu Aug 22 13:51:24 2013 -0300
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Thu Aug 22 22:32:16 2013 +0530
@@ -55,6 +55,7 @@
import javax.script.ScriptEngine;
import javax.script.ScriptEngineFactory;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import jdk.nashorn.internal.runtime.Context;
import jdk.nashorn.internal.runtime.ErrorManager;
import jdk.nashorn.internal.runtime.GlobalObject;
@@ -74,6 +75,11 @@
*/
public final class NashornScriptEngine extends AbstractScriptEngine implements Compilable, Invocable {
+ /**
+ * Key used to associate Nashorn global object mirror with arbitrary Bindings instance.
+ */
+ public static final String NASHORN_GLOBAL = "nashorn.global";
+
private static AccessControlContext createPermAccCtxt(final String permName) {
final Permissions perms = new Permissions();
perms.add(new RuntimePermission(permName));
@@ -85,6 +91,8 @@
private final ScriptEngineFactory factory;
private final Context nashornContext;
+ // do we want to share single Nashorn global instance across ENGINE_SCOPEs?
+ private final boolean _global_per_engine;
private final ScriptObject global;
// initialized bit late to be made 'final'. Property object for "context"
// property of global object
@@ -134,6 +142,9 @@
}
}, CREATE_CONTEXT_ACC_CTXT);
+ // cache this option that is used often
+ this._global_per_engine = nashornContext.getEnv()._global_per_engine;
+
// create new global object
this.global = createNashornGlobal();
// set the default engine scope for the default context
@@ -176,8 +187,14 @@
@Override
public Bindings createBindings() {
- final ScriptObject newGlobal = createNashornGlobal();
- return new ScriptObjectMirror(newGlobal, newGlobal);
+ if (_global_per_engine) {
+ // just create normal SimpleBindings.
+ // We use same 'global' for all Bindings.
+ return new SimpleBindings();
+ } else {
+ final ScriptObject newGlobal = createNashornGlobal();
+ return new ScriptObjectMirror(newGlobal, newGlobal);
+ }
}
// Compilable methods
@@ -319,17 +336,43 @@
}
private ScriptObject getNashornGlobalFrom(final ScriptContext ctxt) {
+ if (_global_per_engine) {
+ // shared single global for all ENGINE_SCOPE Bindings
+ return global;
+ }
+
final Bindings bindings = ctxt.getBindings(ScriptContext.ENGINE_SCOPE);
+ // is this Nashorn's own Bindings implementation?
if (bindings instanceof ScriptObjectMirror) {
- final ScriptObjectMirror mirror = (ScriptObjectMirror)bindings;
- ScriptObject sobj = ((ScriptObjectMirror)bindings).getScriptObject();
- if (sobj instanceof GlobalObject && sobj.isOfContext(nashornContext)) {
+ final ScriptObject sobj = globalFromMirror((ScriptObjectMirror)bindings);
+ if (sobj != null) {
return sobj;
}
}
- // didn't find global object from context given - return the engine-wide global
- return global;
+ // Arbitrary user Bindings implementation. Look for NASHORN_GLOBAL in it!
+ Object scope = bindings.get(NASHORN_GLOBAL);
+ if (scope instanceof ScriptObjectMirror) {
+ final ScriptObject sobj = globalFromMirror((ScriptObjectMirror)scope);
+ if (sobj != null) {
+ return sobj;
+ }
+ }
+
+ // We didn't find associated nashorn global mirror in the Bindings given!
+ // Create new global instance mirror and associate with the Bindings.
+ final ScriptObjectMirror mirror = (ScriptObjectMirror)createBindings();
+ bindings.put(NASHORN_GLOBAL, mirror);
+ return mirror.getScriptObject();
+ }
+
+ private ScriptObject globalFromMirror(final ScriptObjectMirror mirror) {
+ ScriptObject sobj = mirror.getScriptObject();
+ if (sobj instanceof GlobalObject && sobj.isOfContext(nashornContext)) {
+ return sobj;
+ }
+
+ return null;
}
private ScriptObject createNashornGlobal() {
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptEnvironment.java Thu Aug 22 13:51:24 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptEnvironment.java Thu Aug 22 22:32:16 2013 +0530
@@ -86,6 +86,9 @@
/** Launch using as fx application */
public final boolean _fx;
+ /** Use single Global instance per jsr223 engine instance. */
+ public final boolean _global_per_engine;
+
/**
* Behavior when encountering a function declaration in a lexical context where only statements are acceptable
* (function declarations are source elements, but not statements).
@@ -208,6 +211,7 @@
_function_statement = FunctionStatementBehavior.ACCEPT;
}
_fx = options.getBoolean("fx");
+ _global_per_engine = options.getBoolean("global.per.engine");
_lazy_compilation = options.getBoolean("lazy.compilation");
_loader_per_compile = options.getBoolean("loader.per.compile");
_no_java = options.getBoolean("no.java");
--- a/nashorn/src/jdk/nashorn/internal/runtime/resources/Options.properties Thu Aug 22 13:51:24 2013 -0300
+++ b/nashorn/src/jdk/nashorn/internal/runtime/resources/Options.properties Thu Aug 22 22:32:16 2013 +0530
@@ -157,6 +157,14 @@
default=false \
}
+nashorn.option.global.per.engine = { \
+ name="--global-per-engine", \
+ desc="Use single Global instance per script engine instance.", \
+ is_undocumented=true, \
+ type=Boolean, \
+ default=false \
+}
+
nashorn.option.log = { \
name="--log", \
is_undocumented=true, \
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Thu Aug 22 13:51:24 2013 -0300
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Thu Aug 22 22:32:16 2013 +0530
@@ -47,6 +47,7 @@
import javax.script.ScriptEngineFactory;
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import javax.script.SimpleScriptContext;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -1282,4 +1283,50 @@
final Object newObj = ((ScriptObjectMirror)e2obj).newObject("foo");
assertTrue(newObj instanceof ScriptObjectMirror);
}
+
+ @Test
+ public void userEngineScopeBindingsTest() throws ScriptException {
+ final ScriptEngineManager m = new ScriptEngineManager();
+ final ScriptEngine e = m.getEngineByName("nashorn");
+ e.eval("function func() {}");
+
+ final ScriptContext newContext = new SimpleScriptContext();
+ newContext.setBindings(new SimpleBindings(), ScriptContext.ENGINE_SCOPE);
+ // we are using a new bindings - so it should have 'func' defined
+ Object value = e.eval("typeof func", newContext);
+ assertTrue(value.equals("undefined"));
+ }
+
+ @Test
+ public void userEngineScopeBindingsNoLeakTest() throws ScriptException {
+ final ScriptEngineManager m = new ScriptEngineManager();
+ final ScriptEngine e = m.getEngineByName("nashorn");
+ final ScriptContext newContext = new SimpleScriptContext();
+ newContext.setBindings(new SimpleBindings(), ScriptContext.ENGINE_SCOPE);
+ e.eval("function foo() {}", newContext);
+
+ // in the default context's ENGINE_SCOPE, 'foo' shouldn't exist
+ assertTrue(e.eval("typeof foo").equals("undefined"));
+ }
+
+ @Test
+ public void userEngineScopeBindingsRetentionTest() throws ScriptException {
+ final ScriptEngineManager m = new ScriptEngineManager();
+ final ScriptEngine e = m.getEngineByName("nashorn");
+ final ScriptContext newContext = new SimpleScriptContext();
+ newContext.setBindings(new SimpleBindings(), ScriptContext.ENGINE_SCOPE);
+ e.eval("function foo() {}", newContext);
+
+ // definition retained with user's ENGINE_SCOPE Binding
+ assertTrue(e.eval("typeof foo", newContext).equals("function"));
+
+ final Bindings oldBindings = newContext.getBindings(ScriptContext.ENGINE_SCOPE);
+ // but not in another ENGINE_SCOPE binding
+ newContext.setBindings(new SimpleBindings(), ScriptContext.ENGINE_SCOPE);
+ assertTrue(e.eval("typeof foo", newContext).equals("undefined"));
+
+ // restore ENGINE_SCOPE and check again
+ newContext.setBindings(oldBindings, ScriptContext.ENGINE_SCOPE);
+ assertTrue(e.eval("typeof foo", newContext).equals("function"));
+ }
}
--- a/nashorn/test/src/jdk/nashorn/internal/runtime/TrustedScriptEngineTest.java Thu Aug 22 13:51:24 2013 -0300
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/TrustedScriptEngineTest.java Thu Aug 22 22:32:16 2013 +0530
@@ -32,7 +32,10 @@
import javax.script.ScriptEngine;
import javax.script.ScriptEngineFactory;
import javax.script.ScriptEngineManager;
+import javax.script.ScriptContext;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
+import javax.script.SimpleScriptContext;
import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
import org.testng.annotations.Test;
@@ -196,4 +199,25 @@
}
fail("Cannot find nashorn factory!");
}
+
+ @Test
+ public void globalPerEngineTest() throws ScriptException {
+ final NashornScriptEngineFactory fac = new NashornScriptEngineFactory();
+ final String[] options = new String[] { "--global-per-engine" };
+ final ScriptEngine e = fac.getScriptEngine(options);
+
+ e.eval("function foo() {}");
+
+ final ScriptContext newCtx = new SimpleScriptContext();
+ newCtx.setBindings(e.createBindings(), ScriptContext.ENGINE_SCOPE);
+
+ // all global definitions shared and so 'foo' should be
+ // visible in new Bindings as well.
+ assertTrue(e.eval("typeof foo", newCtx).equals("function"));
+
+ e.eval("function bar() {}", newCtx);
+
+ // bar should be visible in default context
+ assertTrue(e.eval("typeof bar").equals("function"));
+ }
}