8023560: Arbitrary javax.script.Bindings objects as ENGINE_SCOPE objects are not handled as expected.
authorsundar
Thu, 22 Aug 2013 22:32:16 +0530
changeset 19623 53207b2e3a7d
parent 19622 b042dad0de96
child 19624 976b3bfbd657
8023560: Arbitrary javax.script.Bindings objects as ENGINE_SCOPE objects are not handled as expected. Reviewed-by: jlaskey, lagergren, hannesw
nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptEnvironment.java
nashorn/src/jdk/nashorn/internal/runtime/resources/Options.properties
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
nashorn/test/src/jdk/nashorn/internal/runtime/TrustedScriptEngineTest.java
--- 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"));
+    }
 }