# HG changeset patch # User sundar # Date 1433236985 -19800 # Node ID b4a5485d6ff31d6b942e1267a4ff6d675a1d182c # Parent b8deeb25baec59d21c69e6da906e88e656730e34 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 diff -r b8deeb25baec -r b4a5485d6ff3 src/jdk/nashorn/api/scripting/NashornScriptEngine.java --- 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"); diff -r b8deeb25baec -r b4a5485d6ff3 src/jdk/nashorn/internal/objects/Global.java --- 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 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) { diff -r b8deeb25baec -r b4a5485d6ff3 src/jdk/nashorn/internal/runtime/Context.java --- 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); } /** diff -r b8deeb25baec -r b4a5485d6ff3 test/src/jdk/nashorn/api/scripting/test/ScopeTest.java --- 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(); + } }