8138616: invokeFunction fails if function calls a function defined in GLOBAL_SCOPE

Reviewed-by: hannesw, mhaupt

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

@@ -140,7 +140,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
         this._global_per_engine = nashornContext.getEnv()._global_per_engine;
 
         // create new global object
-        this.global = createNashornGlobal(context);
+        this.global = createNashornGlobal();
         // set the default ENGINE_SCOPE object for the default context
         context.setBindings(new ScriptObjectMirror(global, global), ScriptContext.ENGINE_SCOPE);
     }
@@ -167,7 +167,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
             // We use same 'global' for all Bindings.
             return new SimpleBindings();
         }
-        return createGlobalMirror(null);
+        return createGlobalMirror();
     }
 
     // Compilable methods
@@ -317,7 +317,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
 
         // 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 = createGlobalMirror(ctxt);
+        final ScriptObjectMirror mirror = createGlobalMirror();
         bindings.put(NASHORN_GLOBAL, mirror);
         return mirror.getHomeGlobal();
     }
@@ -333,13 +333,13 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
     }
 
     // Create a new ScriptObjectMirror wrapping a newly created Nashorn Global object
-    private ScriptObjectMirror createGlobalMirror(final ScriptContext ctxt) {
+    private ScriptObjectMirror createGlobalMirror() {
-        final Global newGlobal = createNashornGlobal(ctxt);
+        final Global newGlobal = createNashornGlobal();
         return new ScriptObjectMirror(newGlobal, newGlobal);
     }
 
     // Create a new Nashorn Global object
-    private Global createNashornGlobal(final ScriptContext ctxt) {
+    private Global createNashornGlobal() {
         final Global newGlobal = AccessController.doPrivileged(new PrivilegedAction<Global>() {
             @Override
             public Global run() {
@@ -354,7 +354,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
             }
         }, CREATE_GLOBAL_ACC_CTXT);
 
-        nashornContext.initGlobal(newGlobal, this, ctxt);
+        nashornContext.initGlobal(newGlobal, this);
 
         return newGlobal;
     }

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

@@ -928,8 +928,6 @@ public final class Global extends Scope {
     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;
@@ -957,7 +955,7 @@ public final class Global extends Scope {
 
     private ScriptContext currentContext() {
         final ScriptContext sc = scontext != null? scontext.get() : null;
-        return sc == null? initscontext : sc;
+        return (sc != null)? sc : (engine != null? engine.getContext() : null);
     }
 
     @Override
@@ -1067,16 +1065,14 @@ public final class Global extends Scope {
      * of the global scope object.
      *
      * @param eng ScriptEngine to initialize
-     * @param ctxt ScriptContext to initialize
      */
-    public void initBuiltinObjects(final ScriptEngine eng, final ScriptContext ctxt) {
+    public void initBuiltinObjects(final ScriptEngine eng) {
         if (this.builtinObject != null) {
             // already initialized, just return
             return;
         }
 
         this.engine = eng;
-        this.initscontext = ctxt;
         if (this.engine != null) {
             this.scontext = new ThreadLocal<>();
         }

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

@@ -1264,17 +1264,16 @@ 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, final ScriptContext ctxt) {
+    public Global initGlobal(final Global global, final ScriptEngine engine) {
         // 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, ctxt);
+                global.initBuiltinObjects(engine);
             } finally {
                 Context.setGlobal(oldGlobal);
             }
@@ -1290,7 +1289,7 @@ public final class Context {
      * @return the initialized global scope object.
      */
     public Global initGlobal(final Global global) {
-        return initGlobal(global, null, null);
+        return initGlobal(global, null);
     }
 
     /**

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

@@ -23,7 +23,7 @@
  * questions.
  */
 
-package jdk.nashorn.api.scripting;
+package jdk.nashorn.api.scripting.test;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
@@ -33,6 +33,9 @@ import java.util.List;
 import java.util.Map;
 import javax.script.ScriptEngine;
 import javax.script.ScriptException;
+import jdk.nashorn.api.scripting.JSObject;
+import jdk.nashorn.api.scripting.NashornScriptEngine;
+import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 

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

@@ -30,6 +30,7 @@ import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 import javax.script.Bindings;
+import javax.script.Invocable;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineFactory;
@@ -855,4 +856,59 @@ public class ScopeTest {
             assertTrue(ret, "Expected true in iteration " + i);
         }
     }
+
+    // @bug 8138616: invokeFunction fails if function calls a function defined in GLOBAL_SCOPE
+    @Test
+    public void invokeFunctionInGlobalScopeTest() throws Exception {
+         final ScriptEngine engine = new ScriptEngineManager().getEngineByName("nashorn");
+         final ScriptContext ctxt = engine.getContext();
+
+         // define a function called "func"
+         engine.eval("func = function() { return 42 }");
+
+         // move ENGINE_SCOPE Bindings to GLOBAL_SCOPE
+         ctxt.setBindings(ctxt.getBindings(ScriptContext.ENGINE_SCOPE), ScriptContext.GLOBAL_SCOPE);
+
+         // create a new Bindings and set as ENGINE_SCOPE
+         ctxt.setBindings(engine.createBindings(), ScriptContext.ENGINE_SCOPE);
+
+         // define new function that calls "func" now in GLOBAL_SCOPE
+         engine.eval("newfunc = function() { return func() }");
+
+         // call "newfunc" and check the return value
+         Object value = ((Invocable)engine).invokeFunction("newfunc");
+         assertTrue(((Number)value).intValue() == 42);
+    }
+
+
+    // @bug 8138616: invokeFunction fails if function calls a function defined in GLOBAL_SCOPE
+    // variant of above that replaces default ScriptContext of the engine with a fresh instance!
+    @Test
+    public void invokeFunctionInGlobalScopeTest2() throws Exception {
+         final ScriptEngine engine = new ScriptEngineManager().getEngineByName("nashorn");
+
+         // create a new ScriptContext instance
+         final ScriptContext ctxt = new SimpleScriptContext();
+         // set it as 'default' ScriptContext
+         engine.setContext(ctxt);
+
+         // create a new Bindings and set as ENGINE_SCOPE
+         ctxt.setBindings(engine.createBindings(), ScriptContext.ENGINE_SCOPE);
+
+         // define a function called "func"
+         engine.eval("func = function() { return 42 }");
+
+         // move ENGINE_SCOPE Bindings to GLOBAL_SCOPE
+         ctxt.setBindings(ctxt.getBindings(ScriptContext.ENGINE_SCOPE), ScriptContext.GLOBAL_SCOPE);
+
+         // create a new Bindings and set as ENGINE_SCOPE
+         ctxt.setBindings(engine.createBindings(), ScriptContext.ENGINE_SCOPE);
+
+         // define new function that calls "func" now in GLOBAL_SCOPE
+         engine.eval("newfunc = function() { return func() }");
+
+         // call "newfunc" and check the return value
+         Object value = ((Invocable)engine).invokeFunction("newfunc");
+         assertTrue(((Number)value).intValue() == 42);
+    }
 }