8166187: Regression: NPE during reparse when using persistent code cache and optimistic types
authorhannesw
Tue, 10 Jan 2017 17:52:29 +0100
changeset 43117 2027548da50f
parent 43116 20c7572ce2a5
child 43118 be8643ebe865
8166187: Regression: NPE during reparse when using persistent code cache and optimistic types Reviewed-by: jlaskey, attila
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java
nashorn/test/src/jdk/nashorn/internal/runtime/test/CodeStoreAndPathTest.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Tue Jan 10 15:27:14 2017 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Tue Jan 10 17:52:29 2017 +0100
@@ -28,6 +28,8 @@
 import static jdk.nashorn.internal.lookup.Lookup.MH;
 
 import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
@@ -100,7 +102,7 @@
      * reparsed from source, or a soft reference to a {@code FunctionNode} for other functions (it is safe
      * to be cleared as they can be reparsed).
      */
-    private volatile Object cachedAst;
+    private volatile transient Object cachedAst;
 
     /** Token of this function within the source. */
     private final long token;
@@ -289,6 +291,9 @@
         if (this.source == null && this.installer == null) {
             this.source    = src;
             this.installer = inst;
+            for (final RecompilableScriptFunctionData nested : nestedFunctions.values()) {
+                nested.initTransients(src, inst);
+            }
         } else if (this.source != src || !this.installer.isCompatibleWith(inst)) {
             // Existing values must be same as those passed as parameters
             throw new IllegalArgumentException();
@@ -424,7 +429,7 @@
         } else if (lCachedAst instanceof SerializedAst) {
             final SerializedAst serializedAst = (SerializedAst)lCachedAst;
             // Even so, are we also softly caching the AST?
-            final FunctionNode cachedFn = serializedAst.cachedAst.get();
+            final FunctionNode cachedFn = serializedAst.cachedAst == null ? null : serializedAst.cachedAst.get();
             if (cachedFn != null) {
                 // Yes we are - this is fast
                 return cloneSymbols(cachedFn);
@@ -492,9 +497,11 @@
      * we're using this tuple instead to also keep a deserialized AST around in memory to cut down on
      * deserialization costs.
      */
-    private static class SerializedAst {
+    private static class SerializedAst implements Serializable {
         private final byte[] serializedAst;
-        private volatile Reference<FunctionNode> cachedAst;
+        private volatile transient Reference<FunctionNode> cachedAst;
+
+        private static final long serialVersionUID = 1L;
 
         SerializedAst(final FunctionNode fn, final Reference<FunctionNode> cachedAst) {
             this.serializedAst = AstSerializer.serialize(fn);
@@ -1038,8 +1045,20 @@
         return true;
     }
 
+    private void writeObject(final ObjectOutputStream out) throws IOException {
+        final Object localCachedAst = cachedAst;
+        out.defaultWriteObject();
+        // We need to persist SerializedAst for split functions as they can't reparse the source code.
+        if (localCachedAst instanceof SerializedAst) {
+            out.writeObject(localCachedAst);
+        } else {
+            out.writeObject(null);
+        }
+    }
+
     private void readObject(final java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
         in.defaultReadObject();
+        cachedAst = in.readObject();
         createLogger();
     }
 
--- a/nashorn/test/src/jdk/nashorn/internal/runtime/test/CodeStoreAndPathTest.java	Tue Jan 10 15:27:14 2017 +0100
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/test/CodeStoreAndPathTest.java	Tue Jan 10 17:52:29 2017 +0100
@@ -47,7 +47,7 @@
 @SuppressWarnings("javadoc")
 public class CodeStoreAndPathTest {
 
-    final String code1 = "var code1; var x = 'Hello Script'; var x1 = 'Hello Script'; "
+    final static String code1 = "var code1; var x = 'Hello Script'; var x1 = 'Hello Script'; "
                 + "var x2 = 'Hello Script'; var x3 = 'Hello Script'; "
                 + "var x4 = 'Hello Script'; var x5 = 'Hello Script';"
                 + "var x6 = 'Hello Script'; var x7 = 'Hello Script'; "
@@ -69,7 +69,7 @@
                 + "x3='Bye Script'; x4='Bye Script'; x5='Bye Script'; x6='Bye Script';"
                 + "x7='Bye Script'; x8='Bye Script'; var x9 = 'Hello Script'; "
                 + "var x10 = 'Hello Script';}";
-    final String code2 = "var code2; var x = 'Hello Script'; var x1 = 'Hello Script'; "
+    final static String code2 = "var code2; var x = 'Hello Script'; var x1 = 'Hello Script'; "
                 + "var x2 = 'Hello Script'; var x3 = 'Hello Script'; "
                 + "var x4 = 'Hello Script'; var x5 = 'Hello Script';"
                 + "var x6 = 'Hello Script'; var x7 = 'Hello Script'; "
@@ -92,9 +92,306 @@
                 + "x7='Bye Script'; x8='Bye Script'; var x9 = 'Hello Script'; "
                 + "var x10 = 'Hello Script';}";
     // Script size < Default minimum size for storing a compiled script class
-    final String code3 = "var code3; var x = 'Hello Script'; var x1 = 'Hello Script'; ";
-    final String codeCache = "build/nashorn_code_cache";
-    final String oldUserDir = System.getProperty("user.dir");
+    final static String code3 = "var code3; var x = 'Hello Script'; var x1 = 'Hello Script'; ";
+    final static String nestedFunctions = "\n" +
+            "(function outer() { \n" +
+            "    var map = null; \n" +
+            "    (function inner() { \n" +
+            "        var object; \n" +
+            "        if (map === null) { \n" +
+            "            map = (function() { \n" +
+            "                var HashMap = Java.type('java.util.HashMap'); \n" +
+            "                map = new HashMap(); \n" +
+            "                map.name          = 'name'; \n" +
+            "                map.id            = 1234;\n" +
+            "                map.basePath      = 'basePath'; \n" +
+            "                map.extensionPath = 'extension';\n" +
+            "                map.address       = 'address'; \n" +
+            "                map.name          = 'name'; \n" +
+            "                map.id            = 1234;\n" +
+            "                map.basePath      = 'basePath'; \n" +
+            "                map.extensionPath = 'extension';\n" +
+            "                map.address       = 'address';\n" +
+            "                map.name          = 'name'; \n" +
+            "                map.id            = 1234;\n" +
+            "                map.basePath      = 'basePath'; \n" +
+            "                map.extensionPath = 'extension';\n" +
+            "                map.address       = 'address'; \n" +
+            "                return map; \n" +
+            "            }()); \n" +
+            "        } \n" +
+            "        object = {}; \n" +
+            "        return object; \n" +
+            "    })(); \n" +
+            "}()); ";
+    final static String longNestedFunctions = "\n" +
+            "(function outer() { \n" +
+            "    var map = null; \n" +
+            "    (function inner() { \n" +
+            "        var object; \n" +
+            "        var HashMap = Java.type('java.util.HashMap'); \n" +
+            "        map = new HashMap(); \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address';\n" +
+            "        map.name          = 'name'; \n" +
+            "        map.id            = 1234;\n" +
+            "        map.basePath      = 'basePath'; \n" +
+            "        map.extensionPath = 'extension';\n" +
+            "        map.address       = 'address'; \n" +
+            "        object = {}; \n" +
+            "        return object; \n" +
+            "    })(); \n" +
+            "}()); ";
+    final static String codeCache = "build/nashorn_code_cache";
+    final static String oldUserDir = System.getProperty("user.dir");
 
     private static final String[] ENGINE_OPTIONS_OPT   = new String[]{"--persistent-code-cache", "--optimistic-types=true"};
     private static final String[] ENGINE_OPTIONS_NOOPT = new String[]{"--persistent-code-cache", "--optimistic-types=false"};
@@ -166,6 +463,28 @@
         checkCompiledScripts(stream, 4);
     }
 
+    @Test
+    public void testNestedFunctionStore() throws ScriptException, IOException {
+        System.setProperty("nashorn.persistent.code.cache", codeCache);
+        final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(nestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(nestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(nestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(nestedFunctions);
+    }
+
+    @Test
+    public void testSplitFunctionStore() throws ScriptException, IOException {
+        System.setProperty("nashorn.persistent.code.cache", codeCache);
+        System.setProperty("nashorn.compiler.splitter.threshold", "500");
+        final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(longNestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(longNestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(longNestedFunctions);
+        factory.getScriptEngine(ENGINE_OPTIONS_OPT).eval(longNestedFunctions);
+        System.getProperties().remove("nashorn.compiler.splitter.threshold");
+    }
+
     private static Path getCodeCachePath(final boolean optimistic) {
         final String codeCache = System.getProperty("nashorn.persistent.code.cache");
         final Path codeCachePath = FileSystems.getDefault().getPath(codeCache).toAbsolutePath();