# HG changeset patch # User sundar # Date 1360831618 -19800 # Node ID efd57dd90de608187bfcc902d4bd50c38d4d61c9 # Parent 9c8790061bee8d015723d278e016f58f892d2d8f 8008197: Cross script engine function calls do not work as expected Reviewed-by: lagergren, hannesw diff -r 9c8790061bee -r efd57dd90de6 nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java --- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Thu Feb 14 09:14:31 2013 +0530 +++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Thu Feb 14 14:16:58 2013 +0530 @@ -329,7 +329,7 @@ final ScriptObject ctxtGlobal = getNashornGlobalFrom(context); final boolean globalChanged = (oldGlobal != ctxtGlobal); - Object self = selfObject; + Object self = globalChanged? ScriptObjectMirror.wrap(selfObject, oldGlobal) : selfObject; try { if (globalChanged) { @@ -354,7 +354,8 @@ if (value instanceof ScriptFunction) { final Object res; try { - res = ScriptRuntime.apply((ScriptFunction)value, self, ScriptObjectMirror.unwrapArray(args, ctxtGlobal)); + final Object[] modArgs = globalChanged? ScriptObjectMirror.wrapArray(args, oldGlobal) : args; + res = ScriptRuntime.checkAndApply((ScriptFunction)value, self, ScriptObjectMirror.unwrapArray(modArgs, ctxtGlobal)); } catch (final Exception e) { throwAsScriptException(e); throw new AssertionError("should not reach here"); diff -r 9c8790061bee -r efd57dd90de6 nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java --- a/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Thu Feb 14 09:14:31 2013 +0530 +++ b/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java Thu Feb 14 14:16:58 2013 +0530 @@ -104,38 +104,30 @@ // JSObject methods @Override public Object call(final String methodName, final Object args[]) { - final Object val = sobj.get(methodName); final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal(); final boolean globalChanged = (oldGlobal != global); - if (val instanceof ScriptFunction) { - final Object[] modifiedArgs = unwrapArray(args, global); - if (modifiedArgs != null) { - for (int i = 0; i < modifiedArgs.length; i++) { - final Object arg = modifiedArgs[i]; - if (arg instanceof ScriptObject) { - modifiedArgs[i] = wrap(arg, oldGlobal); - } - } + try { + if (globalChanged) { + NashornScriptEngine.setNashornGlobal(global); + } + + final Object val = sobj.get(methodName); + if (! (val instanceof ScriptFunction)) { + throw new RuntimeException("No such method: " + methodName); } - try { - if (globalChanged) { - NashornScriptEngine.setNashornGlobal(global); - } - return wrap(((ScriptFunction)val).invoke(sobj, modifiedArgs), global); - } catch (final RuntimeException | Error e) { - throw e; - } catch (final Throwable t) { - throw new RuntimeException(t); - } finally { - if (globalChanged) { - NashornScriptEngine.setNashornGlobal(oldGlobal); - } + final Object[] modArgs = globalChanged? wrapArray(args, oldGlobal) : args; + return wrap(ScriptRuntime.checkAndApply((ScriptFunction)val, sobj, unwrapArray(modArgs, global)), global); + } catch (final RuntimeException | Error e) { + throw e; + } catch (final Throwable t) { + throw new RuntimeException(t); + } finally { + if (globalChanged) { + NashornScriptEngine.setNashornGlobal(oldGlobal); } - } - - throw new RuntimeException("No such method: " + methodName); + } } @Override @@ -180,7 +172,7 @@ @Override public void setMember(final String name, final Object value) { - put(name, wrap(value, NashornScriptEngine.getNashornGlobal())); + put(name, value); } @Override @@ -275,20 +267,27 @@ @Override public Object put(final String key, final Object value) { + final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal(); + final boolean globalChanged = (oldGlobal != global); return inGlobal(new Callable() { @Override public Object call() { - return sobj.put(key, unwrap(value, global)); + final Object modValue = globalChanged? wrap(value, oldGlobal) : value; + return translateUndefined(wrap(sobj.put(key, unwrap(modValue, global)), global)); } }); } @Override public void putAll(final Map map) { + final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal(); + final boolean globalChanged = (oldGlobal != global); final boolean strict = sobj.isStrictContext(); inGlobal(new Callable() { @Override public Object call() { for (final Map.Entry entry : map.entrySet()) { - sobj.set(entry.getKey(), unwrap(entry.getValue(), global), strict); + final Object value = entry.getValue(); + final Object modValue = globalChanged? wrap(value, oldGlobal) : value; + sobj.set(entry.getKey(), unwrap(modValue, global), strict); } return null; } @@ -321,7 +320,7 @@ final Iterator iter = sobj.valueIterator(); while (iter.hasNext()) { - values.add(wrap(iter.next(), global)); + values.add(translateUndefined(wrap(iter.next(), global))); } return Collections.unmodifiableList(values); diff -r 9c8790061bee -r efd57dd90de6 nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java --- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java Thu Feb 14 09:14:31 2013 +0530 +++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java Thu Feb 14 14:16:58 2013 +0530 @@ -192,7 +192,7 @@ * @return ScriptFunction result. * @throws Throwable if there is an exception/error with the invocation or thrown from it */ - public Object invoke(final Object self, final Object... arguments) throws Throwable { + Object invoke(final Object self, final Object... arguments) throws Throwable { if (Context.DEBUG) { invokes++; } diff -r 9c8790061bee -r efd57dd90de6 nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java --- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java Thu Feb 14 09:14:31 2013 +0530 +++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java Thu Feb 14 14:16:58 2013 +0530 @@ -302,6 +302,37 @@ } /** + * Check that the target function is associated with current Context. And also make sure that 'self', if + * ScriptObject, is from current context. + * + * Call a function given self and args. If the number of the arguments is known in advance, you can likely achieve + * better performance by {@link Bootstrap#createDynamicInvoker(String, Class, Class...) creating a dynamic invoker} + * for operation {@code "dyn:call"}, then using its {@link MethodHandle#invokeExact(Object...)} method instead. + * + * @param target ScriptFunction object. + * @param self Receiver in call. + * @param args Call arguments. + * @return Call result. + */ + public static Object checkAndApply(final ScriptFunction target, final Object self, final Object... args) { + final ScriptObject global = Context.getGlobalTrusted(); + if (! (global instanceof GlobalObject)) { + throw new IllegalStateException("No current global set"); + } + + if (target.getContext() != global.getContext()) { + throw new IllegalArgumentException("'target' function is not from current Context"); + } + + if (self instanceof ScriptObject && ((ScriptObject)self).getContext() != global.getContext()) { + throw new IllegalArgumentException("'self' object is not from current Context"); + } + + // all in order - call real 'apply' + return apply(target, self, args); + } + + /** * Call a function given self and args. If the number of the arguments is known in advance, you can likely achieve * better performance by {@link Bootstrap#createDynamicInvoker(String, Class, Class...) creating a dynamic invoker} * for operation {@code "dyn:call"}, then using its {@link MethodHandle#invokeExact(Object...)} method instead. diff -r 9c8790061bee -r efd57dd90de6 nashorn/test/script/basic/JDK-8008197.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/nashorn/test/script/basic/JDK-8008197.js Thu Feb 14 14:16:58 2013 +0530 @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2010, 2013, 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. + * + * 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. + */ + +/** + * JDK-8008197: Cross script engine function calls do not work as expected + * + * @test + * @run + */ + + +var m = new javax.script.ScriptEngineManager(); +var e = m.getEngineByName("nashorn"); + +var obj = { + func: function(str) { + return /hello/.exec(str); + } +}; + +// set our object as object to the engine created +e.put("obj", obj); + +// try to invoke method from the other engine +if (e.eval("obj.func('hello')") == null) { + fail("FAILED: #1 obj.func('hello') returns null"); +} + +// define an object in the engine + e.eval("var foo = { callMe: function(callback) { return callback() } }"); + +// try to invoke a script method from here but callback is from this engine +var res = e.invokeMethod(e.get("foo"), "callMe", function() { + return /nashorn/; +}); + +if (! res.exec("nashorn")) { + fail("FAILED: #2 /nashorn/ does not match 'nashorn'"); +} + +// invoke a script method from here with callback from this engine. +// This uses JSObject.call interface +res = e.get("foo").callMe(function() { + return /ecmascript/; +}); + +if (! res.exec("ecmascript")) { + fail("FAILED: #3 /ecmascript/ does not match 'ecmascript'"); +} diff -r 9c8790061bee -r efd57dd90de6 nashorn/test/script/basic/jquery.js --- a/nashorn/test/script/basic/jquery.js Thu Feb 14 09:14:31 2013 +0530 +++ b/nashorn/test/script/basic/jquery.js Thu Feb 14 14:16:58 2013 +0530 @@ -60,23 +60,13 @@ var name; try { + var split = url.split('/'); + name = split[split.length - 1]; + var path = __DIR__ + "../external/jquery/" + name; try { - var split = url.split('/'); - name = split[split.length - 1]; - var path = __DIR__ + "../external/jquery/" + name; - try { - load(path); - } catch (e) { - checkWindow(e); - } + load(path); } catch (e) { - // try to load it from the internet, if we for some licensing reason - // aren't allowed to ship third party code under MIT license - try { - load(url); - } catch (e) { - checkWindow(e); - } + checkWindow(e); } } catch (e) { print("Unexpected exception " + e); @@ -85,8 +75,6 @@ print("done " + name); } - - for each (url in urls) { test_jquery(url); }