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

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/NashornScriptEngine.java

@@ -354,8 +354,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
             }
         }, CREATE_GLOBAL_ACC_CTXT);
 
-        nashornContext.initGlobal(newGlobal, this);
+        nashornContext.initGlobal(newGlobal, this, ctxt);
-        newGlobal.setScriptContext(ctxt);
 
         return newGlobal;
     }
@@ -404,7 +403,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
         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 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
             }
 
             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 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
         }
     }
 
-    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 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
                 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");

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java

@@ -928,9 +928,11 @@ public final class Global extends ScriptObject implements Scope {
     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 @@ public final class Global extends ScriptObject implements Scope {
 
     /**
      * Set the current script context
-     * @param scontext script context
+     * @param ctxt script context
      */
-    public void setScriptContext(final ScriptContext scontext) {
+    public void setScriptContext(final ScriptContext ctxt) {
-        this.scontext = scontext;
+        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
@@ -1056,14 +1073,19 @@ public final class Global extends ScriptObject implements Scope {
      * of the global scope object.
      *
      * @param eng ScriptEngine to initialize
+     * @param ctxt ScriptContext to initialize
      */
-    public void initBuiltinObjects(final ScriptEngine eng) {
+    public void initBuiltinObjects(final ScriptEngine eng, final ScriptContext ctxt) {
         if (this.builtinObject != null) {
             // already initialized, just return
             return;
         }
 
         this.engine = eng;
+        this.initscontext = ctxt;
+        if (this.engine != null) {
+            this.scontext = new ThreadLocal<>();
+        }
         init(eng);
     }
 
@@ -1392,7 +1414,7 @@ public final class Global extends ScriptObject implements Scope {
      */
     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) {
@@ -2737,8 +2759,9 @@ public final class Global extends ScriptObject implements Scope {
     }
 
     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) {

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java

@@ -66,6 +66,7 @@ import java.util.concurrent.atomic.AtomicReference;
 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 @@ public final class Context {
      *
      * @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 @@ public final class Context {
      * @return the initialized global scope object.
      */
     public Global initGlobal(final Global global) {
-        return initGlobal(global, null);
+        return initGlobal(global, null, null);
     }
 
     /**

nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java

@@ -31,10 +31,12 @@ import static org.testng.Assert.fail;
 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 @@ public class ScopeTest {
             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();
+    }
 }