Tue, 02 Jun 2015 14:53:05 +0530
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
1.1 --- a/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Wed May 27 14:37:11 2015 +0300 1.2 +++ b/src/jdk/nashorn/api/scripting/NashornScriptEngine.java Tue Jun 02 14:53:05 2015 +0530 1.3 @@ -354,8 +354,7 @@ 1.4 } 1.5 }, CREATE_GLOBAL_ACC_CTXT); 1.6 1.7 - nashornContext.initGlobal(newGlobal, this); 1.8 - newGlobal.setScriptContext(ctxt); 1.9 + nashornContext.initGlobal(newGlobal, this, ctxt); 1.10 1.11 return newGlobal; 1.12 } 1.13 @@ -404,7 +403,7 @@ 1.14 return evalImpl(script, ctxt, getNashornGlobalFrom(ctxt)); 1.15 } 1.16 1.17 - private static Object evalImpl(final Context.MultiGlobalCompiledScript mgcs, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException { 1.18 + private Object evalImpl(final Context.MultiGlobalCompiledScript mgcs, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException { 1.19 final Global oldGlobal = Context.getGlobal(); 1.20 final boolean globalChanged = (oldGlobal != ctxtGlobal); 1.21 try { 1.22 @@ -413,8 +412,13 @@ 1.23 } 1.24 1.25 final ScriptFunction script = mgcs.getFunction(ctxtGlobal); 1.26 + final ScriptContext oldCtxt = ctxtGlobal.getScriptContext(); 1.27 ctxtGlobal.setScriptContext(ctxt); 1.28 - return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal)); 1.29 + try { 1.30 + return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal)); 1.31 + } finally { 1.32 + ctxtGlobal.setScriptContext(oldCtxt); 1.33 + } 1.34 } catch (final Exception e) { 1.35 throwAsScriptException(e, ctxtGlobal); 1.36 throw new AssertionError("should not reach here"); 1.37 @@ -425,7 +429,7 @@ 1.38 } 1.39 } 1.40 1.41 - private static Object evalImpl(final ScriptFunction script, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException { 1.42 + private Object evalImpl(final ScriptFunction script, final ScriptContext ctxt, final Global ctxtGlobal) throws ScriptException { 1.43 if (script == null) { 1.44 return null; 1.45 } 1.46 @@ -436,8 +440,13 @@ 1.47 Context.setGlobal(ctxtGlobal); 1.48 } 1.49 1.50 + final ScriptContext oldCtxt = ctxtGlobal.getScriptContext(); 1.51 ctxtGlobal.setScriptContext(ctxt); 1.52 - return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal)); 1.53 + try { 1.54 + return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(ScriptRuntime.apply(script, ctxtGlobal), ctxtGlobal)); 1.55 + } finally { 1.56 + ctxtGlobal.setScriptContext(oldCtxt); 1.57 + } 1.58 } catch (final Exception e) { 1.59 throwAsScriptException(e, ctxtGlobal); 1.60 throw new AssertionError("should not reach here");
2.1 --- a/src/jdk/nashorn/internal/objects/Global.java Wed May 27 14:37:11 2015 +0300 2.2 +++ b/src/jdk/nashorn/internal/objects/Global.java Tue Jun 02 14:53:05 2015 +0530 2.3 @@ -928,9 +928,11 @@ 2.4 private final Context context; 2.5 2.6 // current ScriptContext to use - can be null. 2.7 - private ScriptContext scontext; 2.8 + private ThreadLocal<ScriptContext> scontext; 2.9 // current ScriptEngine associated - can be null. 2.10 private ScriptEngine engine; 2.11 + // initial ScriptContext - can be null 2.12 + private volatile ScriptContext initscontext; 2.13 2.14 // ES6 global lexical scope. 2.15 private final LexicalScope lexicalScope; 2.16 @@ -940,10 +942,25 @@ 2.17 2.18 /** 2.19 * Set the current script context 2.20 - * @param scontext script context 2.21 + * @param ctxt script context 2.22 */ 2.23 - public void setScriptContext(final ScriptContext scontext) { 2.24 - this.scontext = scontext; 2.25 + public void setScriptContext(final ScriptContext ctxt) { 2.26 + assert scontext != null; 2.27 + scontext.set(ctxt); 2.28 + } 2.29 + 2.30 + /** 2.31 + * Get the current script context 2.32 + * @return current script context 2.33 + */ 2.34 + public ScriptContext getScriptContext() { 2.35 + assert scontext != null; 2.36 + return scontext.get(); 2.37 + } 2.38 + 2.39 + private ScriptContext currentContext() { 2.40 + final ScriptContext sc = scontext != null? scontext.get() : null; 2.41 + return sc == null? initscontext : sc; 2.42 } 2.43 2.44 @Override 2.45 @@ -1055,17 +1072,21 @@ 2.46 * as well as our extension builtin objects like "Java", "JSAdapter" as properties 2.47 * of the global scope object. 2.48 * 2.49 - * @param engine ScriptEngine to initialize 2.50 + * @param eng ScriptEngine to initialize 2.51 + * @param ctxt ScriptContext to initialize 2.52 */ 2.53 - @SuppressWarnings("hiding") 2.54 - public void initBuiltinObjects(final ScriptEngine engine) { 2.55 + public void initBuiltinObjects(final ScriptEngine eng, final ScriptContext ctxt) { 2.56 if (this.builtinObject != null) { 2.57 // already initialized, just return 2.58 return; 2.59 } 2.60 2.61 - this.engine = engine; 2.62 - init(engine); 2.63 + this.engine = eng; 2.64 + this.initscontext = ctxt; 2.65 + if (this.engine != null) { 2.66 + this.scontext = new ThreadLocal<>(); 2.67 + } 2.68 + init(eng); 2.69 } 2.70 2.71 /** 2.72 @@ -1393,7 +1414,7 @@ 2.73 */ 2.74 public static Object __noSuchProperty__(final Object self, final Object name) { 2.75 final Global global = Global.instance(); 2.76 - final ScriptContext sctxt = global.scontext; 2.77 + final ScriptContext sctxt = global.currentContext(); 2.78 final String nameStr = name.toString(); 2.79 2.80 if (sctxt != null) { 2.81 @@ -2485,7 +2506,7 @@ 2.82 } 2.83 2.84 @SuppressWarnings("hiding") 2.85 - private void init(final ScriptEngine engine) { 2.86 + private void init(final ScriptEngine eng) { 2.87 assert Context.getGlobal() == this : "this global is not set as current"; 2.88 2.89 final ScriptEnvironment env = getContext().getEnv(); 2.90 @@ -2601,7 +2622,7 @@ 2.91 addOwnProperty("$ARG", Attribute.NOT_ENUMERABLE, arguments); 2.92 } 2.93 2.94 - if (engine != null) { 2.95 + if (eng != null) { 2.96 // default file name 2.97 addOwnProperty(ScriptEngine.FILENAME, Attribute.NOT_ENUMERABLE, null); 2.98 // __noSuchProperty__ hook for ScriptContext search of missing variables 2.99 @@ -2739,8 +2760,9 @@ 2.100 } 2.101 2.102 private Object printImpl(final boolean newLine, final Object... objects) { 2.103 + final ScriptContext sc = currentContext(); 2.104 @SuppressWarnings("resource") 2.105 - final PrintWriter out = scontext != null? new PrintWriter(scontext.getWriter()) : getContext().getEnv().getOut(); 2.106 + final PrintWriter out = sc != null? new PrintWriter(sc.getWriter()) : getContext().getEnv().getOut(); 2.107 final StringBuilder sb = new StringBuilder(); 2.108 2.109 for (final Object obj : objects) {
3.1 --- a/src/jdk/nashorn/internal/runtime/Context.java Wed May 27 14:37:11 2015 +0300 3.2 +++ b/src/jdk/nashorn/internal/runtime/Context.java Tue Jun 02 14:53:05 2015 +0530 3.3 @@ -66,6 +66,7 @@ 3.4 import java.util.function.Consumer; 3.5 import java.util.function.Supplier; 3.6 import java.util.logging.Level; 3.7 +import javax.script.ScriptContext; 3.8 import javax.script.ScriptEngine; 3.9 import jdk.internal.org.objectweb.asm.ClassReader; 3.10 import jdk.internal.org.objectweb.asm.util.CheckClassAdapter; 3.11 @@ -1095,16 +1096,17 @@ 3.12 * 3.13 * @param global the global 3.14 * @param engine the associated ScriptEngine instance, can be null 3.15 + * @param ctxt the initial ScriptContext, can be null 3.16 * @return the initialized global scope object. 3.17 */ 3.18 - public Global initGlobal(final Global global, final ScriptEngine engine) { 3.19 + public Global initGlobal(final Global global, final ScriptEngine engine, final ScriptContext ctxt) { 3.20 // Need only minimal global object, if we are just compiling. 3.21 if (!env._compile_only) { 3.22 final Global oldGlobal = Context.getGlobal(); 3.23 try { 3.24 Context.setGlobal(global); 3.25 // initialize global scope with builtin global objects 3.26 - global.initBuiltinObjects(engine); 3.27 + global.initBuiltinObjects(engine, ctxt); 3.28 } finally { 3.29 Context.setGlobal(oldGlobal); 3.30 } 3.31 @@ -1120,7 +1122,7 @@ 3.32 * @return the initialized global scope object. 3.33 */ 3.34 public Global initGlobal(final Global global) { 3.35 - return initGlobal(global, null); 3.36 + return initGlobal(global, null, null); 3.37 } 3.38 3.39 /**
4.1 --- a/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Wed May 27 14:37:11 2015 +0300 4.2 +++ b/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Tue Jun 02 14:53:05 2015 +0530 4.3 @@ -31,10 +31,12 @@ 4.4 import javax.script.Bindings; 4.5 import javax.script.ScriptContext; 4.6 import javax.script.ScriptEngine; 4.7 +import javax.script.ScriptEngineFactory; 4.8 import javax.script.ScriptEngineManager; 4.9 import javax.script.ScriptException; 4.10 import javax.script.SimpleBindings; 4.11 import javax.script.SimpleScriptContext; 4.12 +import jdk.nashorn.api.scripting.NashornScriptEngineFactory; 4.13 import jdk.nashorn.api.scripting.ScriptObjectMirror; 4.14 import jdk.nashorn.api.scripting.URLReader; 4.15 import org.testng.Assert; 4.16 @@ -778,4 +780,44 @@ 4.17 throw new AssertionError("should have thrown NPE"); 4.18 } catch (NullPointerException npe5) {} 4.19 } 4.20 + 4.21 + public static class RecursiveEval { 4.22 + private final ScriptEngineFactory factory = new NashornScriptEngineFactory(); 4.23 + private final ScriptEngine engine = factory.getScriptEngine(); 4.24 + private final Bindings engineBindings = engine.getBindings(ScriptContext.ENGINE_SCOPE); 4.25 + 4.26 + public void program() throws ScriptException { 4.27 + ScriptContext sc = new SimpleScriptContext(); 4.28 + Bindings global = new SimpleBindings(); 4.29 + sc.setBindings(global, ScriptContext.GLOBAL_SCOPE); 4.30 + sc.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE); 4.31 + global.put("text", "programText"); 4.32 + String value = engine.eval("text", sc).toString(); 4.33 + Assert.assertEquals(value, "programText"); 4.34 + engine.put("program", this); 4.35 + engine.eval("program.method()"); 4.36 + // eval again from here! 4.37 + value = engine.eval("text", sc).toString(); 4.38 + Assert.assertEquals(value, "programText"); 4.39 + } 4.40 + 4.41 + public void method() throws ScriptException { 4.42 + // a context with a new global bindings, same engine bindings 4.43 + final ScriptContext sc = new SimpleScriptContext(); 4.44 + final Bindings global = new SimpleBindings(); 4.45 + sc.setBindings(global, ScriptContext.GLOBAL_SCOPE); 4.46 + sc.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE); 4.47 + global.put("text", "methodText"); 4.48 + String value = engine.eval("text", sc).toString(); 4.49 + Assert.assertEquals(value, "methodText"); 4.50 + } 4.51 + } 4.52 + 4.53 + // @bug 8081609: engine.eval call from a java method which 4.54 + // was called from a previous engine.eval results in wrong 4.55 + // ScriptContext being used. 4.56 + @Test 4.57 + public void recursiveEvalCallScriptContextTest() throws ScriptException { 4.58 + new RecursiveEval().program(); 4.59 + } 4.60 }