8081609: engine.eval call from a java method which was called from a previous engine.eval results in wrong ScriptContext being used.

Tue, 02 Jun 2015 14:53:05 +0530

author
sundar
date
Tue, 02 Jun 2015 14:53:05 +0530
changeset 1385
b4a5485d6ff3
parent 1384
b8deeb25baec
child 1386
e5b03cc6f269
child 1387
4632d53923d4

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

src/jdk/nashorn/api/scripting/NashornScriptEngine.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/objects/Global.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/Context.java file | annotate | diff | comparison | revisions
test/src/jdk/nashorn/api/scripting/test/ScopeTest.java file | annotate | diff | comparison | revisions
     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  }

mercurial