changeset 1404:b4a5485d6ff3

8081609: engine.eval call from a java method which was called from a previous engine.eval results in wrong ScriptContext being used. Reviewed-by: attila, lagergren
author sundar
date Tue, 02 Jun 2015 14:53:05 +0530
parents b8deeb25baec
children e5b03cc6f269 4632d53923d4
files src/jdk/nashorn/api/scripting/NashornScriptEngine.java src/jdk/nashorn/internal/objects/Global.java src/jdk/nashorn/internal/runtime/Context.java test/src/jdk/nashorn/api/scripting/test/ScopeTest.java
diffstat 4 files changed, 97 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed May 27 14:37:11 2015 +0300
+++ b/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Tue Jun 02 14:53:05 2015 +0530
@@ -354,8 +354,7 @@
             }
         }, CREATE_GLOBAL_ACC_CTXT);
 
-        nashornContext.initGlobal(newGlobal, this);
-        newGlobal.setScriptContext(ctxt);
+        nashornContext.initGlobal(newGlobal, this, ctxt);
 
         return newGlobal;
     }
@@ -404,7 +403,7 @@
         return evalImpl(script, ctxt, getNashornGlobalFrom(ctxt));
     }
 
-    private static Object evalImpl(final Context.MultiGlobalCompiledScript mgcs, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException {
+    private Object evalImpl(final Context.MultiGlobalCompiledScript mgcs, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException {
         final Global oldGlobal = Context.getGlobal();
         final boolean globalChanged = (oldGlobal != ctxtGlobal);
         try {
@@ -413,8 +412,13 @@
             }
 
             final ScriptFunction script = mgcs.getFunction(ctxtGlobal);
+            final ScriptContext oldCtxt = ctxtGlobal.getScriptContext();
             ctxtGlobal.setScriptContext(ctxt);
-            return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal));
+            try {
+                return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal));
+            } finally {
+                ctxtGlobal.setScriptContext(oldCtxt);
+            }
         } catch (final Exception e) {
             throwAsScriptException(e, ctxtGlobal);
             throw new AssertionError("should not reach here");
@@ -425,7 +429,7 @@
         }
     }
 
-    private static Object evalImpl(final ScriptFunction script, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException {
+    private Object evalImpl(final ScriptFunction script, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException {
         if (script == null) {
             return null;
         }
@@ -436,8 +440,13 @@
                 Context.setGlobal(ctxtGlobal);
             }
 
+            final ScriptContext oldCtxt = ctxtGlobal.getScriptContext();
             ctxtGlobal.setScriptContext(ctxt);
-            return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal));
+            try {
+                return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal));
+            } finally {
+                ctxtGlobal.setScriptContext(oldCtxt);
+            }
         } catch (final Exception e) {
             throwAsScriptException(e, ctxtGlobal);
             throw new AssertionError("should not reach here");
--- a/src/jdk/nashorn/internal/objects/Global.java	Wed May 27 14:37:11 2015 +0300
+++ b/src/jdk/nashorn/internal/objects/Global.java	Tue Jun 02 14:53:05 2015 +0530
@@ -928,9 +928,11 @@
     private final Context context;
 
     // current ScriptContext to use - can be null.
-    private ScriptContext scontext;
+    private ThreadLocal<ScriptContext> scontext;
     // current ScriptEngine associated - can be null.
     private ScriptEngine engine;
+    // initial ScriptContext - can be null
+    private volatile ScriptContext initscontext;
 
     // ES6 global lexical scope.
     private final LexicalScope lexicalScope;
@@ -940,10 +942,25 @@
 
     /**
      * Set the current script context
-     * @param scontext script context
+     * @param ctxt script context
      */
-    public void setScriptContext(final ScriptContext scontext) {
-        this.scontext = scontext;
+    public void setScriptContext(final ScriptContext ctxt) {
+        assert scontext != null;
+        scontext.set(ctxt);
+    }
+
+    /**
+     * Get the current script context
+     * @return current script context
+     */
+    public ScriptContext getScriptContext() {
+        assert scontext != null;
+        return scontext.get();
+    }
+
+    private ScriptContext currentContext() {
+        final ScriptContext sc = scontext != null? scontext.get() : null;
+        return sc == null? initscontext : sc;
     }
 
     @Override
@@ -1055,17 +1072,21 @@
      * as well as our extension builtin objects like "Java", "JSAdapter" as properties
      * of the global scope object.
      *
-     * @param engine ScriptEngine to initialize
+     * @param eng ScriptEngine to initialize
+     * @param ctxt ScriptContext to initialize
      */
-    @SuppressWarnings("hiding")
-    public void initBuiltinObjects(final ScriptEngine engine) {
+    public void initBuiltinObjects(final ScriptEngine eng, final ScriptContext ctxt) {
         if (this.builtinObject != null) {
             // already initialized, just return
             return;
         }
 
-        this.engine = engine;
-        init(engine);
+        this.engine = eng;
+        this.initscontext = ctxt;
+        if (this.engine != null) {
+            this.scontext = new ThreadLocal<>();
+        }
+        init(eng);
     }
 
     /**
@@ -1393,7 +1414,7 @@
      */
     public static Object __noSuchProperty__(final Object self, final Object name) {
         final Global global = Global.instance();
-        final ScriptContext sctxt = global.scontext;
+        final ScriptContext sctxt = global.currentContext();
         final String nameStr = name.toString();
 
         if (sctxt != null) {
@@ -2485,7 +2506,7 @@
     }
 
     @SuppressWarnings("hiding")
-    private void init(final ScriptEngine engine) {
+    private void init(final ScriptEngine eng) {
         assert Context.getGlobal() == this : "this global is not set as current";
 
         final ScriptEnvironment env = getContext().getEnv();
@@ -2601,7 +2622,7 @@
             addOwnProperty("$ARG", Attribute.NOT_ENUMERABLE, arguments);
         }
 
-        if (engine != null) {
+        if (eng != null) {
             // default file name
             addOwnProperty(ScriptEngine.FILENAME, Attribute.NOT_ENUMERABLE, null);
             // __noSuchProperty__ hook for ScriptContext search of missing variables
@@ -2739,8 +2760,9 @@
     }
 
     private Object printImpl(final boolean newLine, final Object... objects) {
+        final ScriptContext sc = currentContext();
         @SuppressWarnings("resource")
-        final PrintWriter out = scontext != null? new PrintWriter(scontext.getWriter()) : getContext().getEnv().getOut();
+        final PrintWriter out = sc != null? new PrintWriter(sc.getWriter()) : getContext().getEnv().getOut();
         final StringBuilder sb = new StringBuilder();
 
         for (final Object obj : objects) {
--- a/src/jdk/nashorn/internal/runtime/Context.java	Wed May 27 14:37:11 2015 +0300
+++ b/src/jdk/nashorn/internal/runtime/Context.java	Tue Jun 02 14:53:05 2015 +0530
@@ -66,6 +66,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.logging.Level;
+import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import jdk.internal.org.objectweb.asm.ClassReader;
 import jdk.internal.org.objectweb.asm.util.CheckClassAdapter;
@@ -1095,16 +1096,17 @@
      *
      * @param global the global
      * @param engine the associated ScriptEngine instance, can be null
+     * @param ctxt the initial ScriptContext, can be null
      * @return the initialized global scope object.
      */
-    public Global initGlobal(final Global global, final ScriptEngine engine) {
+    public Global initGlobal(final Global global, final ScriptEngine engine, final ScriptContext ctxt) {
         // Need only minimal global object, if we are just compiling.
         if (!env._compile_only) {
             final Global oldGlobal = Context.getGlobal();
             try {
                 Context.setGlobal(global);
                 // initialize global scope with builtin global objects
-                global.initBuiltinObjects(engine);
+                global.initBuiltinObjects(engine, ctxt);
             } finally {
                 Context.setGlobal(oldGlobal);
             }
@@ -1120,7 +1122,7 @@
      * @return the initialized global scope object.
      */
     public Global initGlobal(final Global global) {
-        return initGlobal(global, null);
+        return initGlobal(global, null, null);
     }
 
     /**
--- a/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java	Wed May 27 14:37:11 2015 +0300
+++ b/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java	Tue Jun 02 14:53:05 2015 +0530
@@ -31,10 +31,12 @@
 import javax.script.Bindings;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
+import javax.script.ScriptEngineFactory;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
 import javax.script.SimpleBindings;
 import javax.script.SimpleScriptContext;
+import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
 import jdk.nashorn.api.scripting.ScriptObjectMirror;
 import jdk.nashorn.api.scripting.URLReader;
 import org.testng.Assert;
@@ -778,4 +780,44 @@
             throw new AssertionError("should have thrown NPE");
         } catch (NullPointerException npe5) {}
     }
+
+    public static class RecursiveEval {
+        private final ScriptEngineFactory factory = new NashornScriptEngineFactory();
+        private final ScriptEngine engine = factory.getScriptEngine();
+        private final Bindings engineBindings = engine.getBindings(ScriptContext.ENGINE_SCOPE);
+
+        public void program() throws ScriptException {
+            ScriptContext sc = new SimpleScriptContext();
+            Bindings global = new SimpleBindings();
+            sc.setBindings(global, ScriptContext.GLOBAL_SCOPE);
+            sc.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE);
+            global.put("text", "programText");
+            String value = engine.eval("text", sc).toString();
+            Assert.assertEquals(value, "programText");
+            engine.put("program", this);
+            engine.eval("program.method()");
+            // eval again from here!
+            value = engine.eval("text", sc).toString();
+            Assert.assertEquals(value, "programText");
+        }
+
+        public void method() throws ScriptException {
+            // a context with a new global bindings, same engine bindings
+            final ScriptContext sc = new SimpleScriptContext();
+            final Bindings global = new SimpleBindings();
+            sc.setBindings(global, ScriptContext.GLOBAL_SCOPE);
+            sc.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE);
+            global.put("text", "methodText");
+            String value = engine.eval("text", sc).toString();
+            Assert.assertEquals(value, "methodText");
+        }
+    }
+
+    // @bug 8081609: engine.eval call from a java method which
+    // was called from a previous engine.eval results in wrong
+    // ScriptContext being used.
+    @Test
+    public void recursiveEvalCallScriptContextTest() throws ScriptException {
+        new RecursiveEval().program();
+    }
 }