8138616: invokeFunction fails if function calls a function defined in GLOBAL_SCOPE
authorsundar
Thu, 01 Oct 2015 21:27:30 +0530
changeset 32893 665eb8283882
parent 32892 7186b06a72cf
child 32894 4f60fd66fe73
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
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java
nashorn/test/src/jdk/nashorn/api/scripting/JSONCompatibleTest.java
nashorn/test/src/jdk/nashorn/api/scripting/test/JSONCompatibleTest.java
nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Oct 01 10:37:25 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Oct 01 21:27:30 2015 +0530
@@ -140,7 +140,7 @@
         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 @@
             // We use same 'global' for all Bindings.
             return new SimpleBindings();
         }
-        return createGlobalMirror(null);
+        return createGlobalMirror();
     }
 
     // Compilable methods
@@ -317,7 +317,7 @@
 
         // 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 @@
     }
 
     // Create a new ScriptObjectMirror wrapping a newly created Nashorn Global object
-    private ScriptObjectMirror createGlobalMirror(final ScriptContext ctxt) {
-        final Global newGlobal = createNashornGlobal(ctxt);
+    private ScriptObjectMirror createGlobalMirror() {
+        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 @@
             }
         }, CREATE_GLOBAL_ACC_CTXT);
 
-        nashornContext.initGlobal(newGlobal, this, ctxt);
+        nashornContext.initGlobal(newGlobal, this);
 
         return newGlobal;
     }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java	Thu Oct 01 10:37:25 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java	Thu Oct 01 21:27:30 2015 +0530
@@ -928,8 +928,6 @@
     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 @@
 
     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 @@
      * 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<>();
         }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java	Thu Oct 01 10:37:25 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java	Thu Oct 01 21:27:30 2015 +0530
@@ -1264,17 +1264,16 @@
      *
      * @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 @@
      * @return the initialized global scope object.
      */
     public Global initGlobal(final Global global) {
-        return initGlobal(global, null, null);
+        return initGlobal(global, null);
     }
 
     /**
--- a/nashorn/test/src/jdk/nashorn/api/scripting/JSONCompatibleTest.java	Thu Oct 01 10:37:25 2015 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,117 +0,0 @@
-/*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.  Oracle designates this
- * particular file as subject to the "Classpath" exception as provided
- * by Oracle in the LICENSE file that accompanied this code.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-package jdk.nashorn.api.scripting;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
-
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import javax.script.ScriptEngine;
-import javax.script.ScriptException;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
-public class JSONCompatibleTest {
-
-    /**
-     * Wrap a top-level array as a list.
-     */
-    @Test
-    public void testWrapArray() throws ScriptException {
-        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
-        final Object val = engine.eval("Java.asJSONCompatible([1, 2, 3])");
-        assertEquals(asList(val), Arrays.asList(1, 2, 3));
-    }
-
-    /**
-     * Wrap an embedded array as a list.
-     */
-    @Test
-    public void testWrapObjectWithArray() throws ScriptException {
-        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
-        final Object val = engine.eval("Java.asJSONCompatible({x: [1, 2, 3]})");
-        assertEquals(asList(asMap(val).get("x")), Arrays.asList(1, 2, 3));
-    }
-
-    /**
-     * Check it all works transitively several more levels down.
-     */
-    @Test
-    public void testDeepWrapping() throws ScriptException {
-        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
-        final Object val = engine.eval("Java.asJSONCompatible({x: [1, {y: [2, {z: [3]}]}, [4, 5]]})");
-        final Map<String, Object> root = asMap(val);
-        final List<Object> x = asList(root.get("x"));
-        assertEquals(x.get(0), 1);
-        final Map<String, Object> x1 = asMap(x.get(1));
-        final List<Object> y = asList(x1.get("y"));
-        assertEquals(y.get(0), 2);
-        final Map<String, Object> y1 = asMap(y.get(1));
-        assertEquals(asList(y1.get("z")), Arrays.asList(3));
-        assertEquals(asList(x.get(2)), Arrays.asList(4, 5));
-    }
-
-    /**
-     * Ensure that the old behaviour (every object is a Map) is unchanged.
-     */
-    @Test
-    public void testNonWrapping() throws ScriptException {
-        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
-        final Object val = engine.eval("({x: [1, {y: [2, {z: [3]}]}, [4, 5]]})");
-        final Map<String, Object> root = asMap(val);
-        final Map<String, Object> x = asMap(root.get("x"));
-        assertEquals(x.get("0"), 1);
-        final Map<String, Object> x1 = asMap(x.get("1"));
-        final Map<String, Object> y = asMap(x1.get("y"));
-        assertEquals(y.get("0"), 2);
-        final Map<String, Object> y1 = asMap(y.get("1"));
-        final Map<String, Object> z = asMap(y1.get("z"));
-        assertEquals(z.get("0"), 3);
-        final Map<String, Object> x2 = asMap(x.get("2"));
-        assertEquals(x2.get("0"), 4);
-        assertEquals(x2.get("1"), 5);
-    }
-
-    @SuppressWarnings("unchecked")
-    private static List<Object> asList(final Object obj) {
-        assertJSObject(obj);
-        Assert.assertTrue(obj instanceof List);
-        return (List)obj;
-    }
-
-    @SuppressWarnings("unchecked")
-    private static Map<String, Object> asMap(final Object obj) {
-        assertJSObject(obj);
-        Assert.assertTrue(obj instanceof Map);
-        return (Map)obj;
-    }
-
-    private static void assertJSObject(final Object obj) {
-        assertTrue(obj instanceof JSObject);
-    }
-}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/test/JSONCompatibleTest.java	Thu Oct 01 21:27:30 2015 +0530
@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.nashorn.api.scripting.test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Arrays;
+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;
+
+public class JSONCompatibleTest {
+
+    /**
+     * Wrap a top-level array as a list.
+     */
+    @Test
+    public void testWrapArray() throws ScriptException {
+        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
+        final Object val = engine.eval("Java.asJSONCompatible([1, 2, 3])");
+        assertEquals(asList(val), Arrays.asList(1, 2, 3));
+    }
+
+    /**
+     * Wrap an embedded array as a list.
+     */
+    @Test
+    public void testWrapObjectWithArray() throws ScriptException {
+        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
+        final Object val = engine.eval("Java.asJSONCompatible({x: [1, 2, 3]})");
+        assertEquals(asList(asMap(val).get("x")), Arrays.asList(1, 2, 3));
+    }
+
+    /**
+     * Check it all works transitively several more levels down.
+     */
+    @Test
+    public void testDeepWrapping() throws ScriptException {
+        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
+        final Object val = engine.eval("Java.asJSONCompatible({x: [1, {y: [2, {z: [3]}]}, [4, 5]]})");
+        final Map<String, Object> root = asMap(val);
+        final List<Object> x = asList(root.get("x"));
+        assertEquals(x.get(0), 1);
+        final Map<String, Object> x1 = asMap(x.get(1));
+        final List<Object> y = asList(x1.get("y"));
+        assertEquals(y.get(0), 2);
+        final Map<String, Object> y1 = asMap(y.get(1));
+        assertEquals(asList(y1.get("z")), Arrays.asList(3));
+        assertEquals(asList(x.get(2)), Arrays.asList(4, 5));
+    }
+
+    /**
+     * Ensure that the old behaviour (every object is a Map) is unchanged.
+     */
+    @Test
+    public void testNonWrapping() throws ScriptException {
+        final ScriptEngine engine = new NashornScriptEngineFactory().getScriptEngine();
+        final Object val = engine.eval("({x: [1, {y: [2, {z: [3]}]}, [4, 5]]})");
+        final Map<String, Object> root = asMap(val);
+        final Map<String, Object> x = asMap(root.get("x"));
+        assertEquals(x.get("0"), 1);
+        final Map<String, Object> x1 = asMap(x.get("1"));
+        final Map<String, Object> y = asMap(x1.get("y"));
+        assertEquals(y.get("0"), 2);
+        final Map<String, Object> y1 = asMap(y.get("1"));
+        final Map<String, Object> z = asMap(y1.get("z"));
+        assertEquals(z.get("0"), 3);
+        final Map<String, Object> x2 = asMap(x.get("2"));
+        assertEquals(x2.get("0"), 4);
+        assertEquals(x2.get("1"), 5);
+    }
+
+    @SuppressWarnings("unchecked")
+    private static List<Object> asList(final Object obj) {
+        assertJSObject(obj);
+        Assert.assertTrue(obj instanceof List);
+        return (List)obj;
+    }
+
+    @SuppressWarnings("unchecked")
+    private static Map<String, Object> asMap(final Object obj) {
+        assertJSObject(obj);
+        Assert.assertTrue(obj instanceof Map);
+        return (Map)obj;
+    }
+
+    private static void assertJSObject(final Object obj) {
+        assertTrue(obj instanceof JSObject);
+    }
+}
--- a/nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java	Thu Oct 01 10:37:25 2015 +0200
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java	Thu Oct 01 21:27:30 2015 +0530
@@ -30,6 +30,7 @@
 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 @@
             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);
+    }
 }