8209055: c.s.t.javac.code.DeferredCompletionFailureHandler seems to use WeakHashMap incorrectly
authorjlahoda
Mon, 12 Nov 2018 09:35:23 +0000
changeset 52489 1a6aeca4a8c9
parent 52488 43081c586d77
child 52490 61915e1458bc
8209055: c.s.t.javac.code.DeferredCompletionFailureHandler seems to use WeakHashMap incorrectly Summary: Do not keep speculative Symbols in DeferredCompletionFailureHandler. Reviewed-by: jjg, vromero
src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java
src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java
test/langtools/tools/javac/processing/model/completionfailure/SymbolsDontCumulate.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java	Mon Nov 12 01:15:16 2018 -0800
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java	Mon Nov 12 09:35:23 2018 +0000
@@ -432,7 +432,10 @@
             try {
                 c.complete();
             } catch (CompletionFailure ex) {
-                if (absent) syms.removeClass(ps.modle, flatname);
+                if (absent) {
+                    syms.removeClass(ps.modle, flatname);
+                    ex.dcfh.classSymbolRemoved(c);
+                }
                 throw ex;
             }
         }
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java	Mon Nov 12 01:15:16 2018 -0800
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java	Mon Nov 12 09:35:23 2018 +0000
@@ -24,8 +24,8 @@
  */
 package com.sun.tools.javac.code;
 
+import java.util.HashMap;
 import java.util.Map;
-import java.util.WeakHashMap;
 
 import com.sun.tools.javac.code.Kinds.Kind;
 import com.sun.tools.javac.code.Scope.WriteableScope;
@@ -62,7 +62,7 @@
     }
 
     public final Handler userCodeHandler = new Handler() {
-        private final Map<ClassSymbol, FlipSymbolDescription> class2Flip = new WeakHashMap<>();
+        private final Map<ClassSymbol, FlipSymbolDescription> class2Flip = new HashMap<>();
 
         public void install() {
             class2Flip.values().forEach(f -> f.flip());
@@ -78,6 +78,9 @@
                 }
             }));
         }
+        public void classSymbolRemoved(ClassSymbol sym) {
+            class2Flip.remove(sym);
+        }
         public void uninstall() {
             class2Flip.values().forEach(f -> f.flip());
         }
@@ -90,6 +93,7 @@
             throw cf;
         }
         public void classSymbolCompleteFailed(ClassSymbol sym, Completer origCompleter) {}
+        public void classSymbolRemoved(ClassSymbol sym) {}
         public void uninstall() {
         }
     };
@@ -118,6 +122,10 @@
         handler.classSymbolCompleteFailed(sym, origCompleter);
     }
 
+    public void classSymbolRemoved(ClassSymbol sym) {
+        handler.classSymbolRemoved(sym);
+    }
+
     public boolean isDeferredCompleter(Completer c) {
         return c instanceof DeferredCompleter;
     }
@@ -126,6 +134,7 @@
         public void install();
         public void handleAPICompletionFailure(CompletionFailure cf);
         public void classSymbolCompleteFailed(ClassSymbol sym, Completer origCompleter);
+        public void classSymbolRemoved(ClassSymbol sym);
         public void uninstall();
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/processing/model/completionfailure/SymbolsDontCumulate.java	Mon Nov 12 09:35:23 2018 +0000
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+/*
+ * @test
+ * @bug 8209055
+ * @summary Verify that speculative symbols are not unnecesarily retained in
+ *          the DeferredCompletionFailureHandler
+ * @library /tools/javac/lib /tools/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.code:+open
+ * @run main SymbolsDontCumulate
+ */
+
+import java.lang.reflect.Field;
+import java.util.*;
+
+import javax.tools.JavaCompiler;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.ToolProvider;
+
+import toolbox.*;
+import com.sun.tools.javac.api.JavacTaskImpl;
+import com.sun.tools.javac.code.DeferredCompletionFailureHandler;
+
+public class SymbolsDontCumulate {
+    ToolBox tb = new ToolBox();
+
+    void testSymbolsCumulate() throws Exception {
+        JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+        List<String> errors = new ArrayList<>();
+
+        try (StandardJavaFileManager fm = compiler.getStandardFileManager(null, null, null)) {
+            com.sun.source.util.JavacTask task = (com.sun.source.util.JavacTask)
+                    compiler.getTask(null,
+                                     fm,
+                                     d -> errors.add(d.getCode()),
+                                     Arrays.asList("-proc:none"),
+                                     Arrays.asList("java.lang.Object"),
+                                     null);
+            assertNotNull(task.getElements().getTypeElement("java.lang.Object"));
+            assertNull(task.getElements().getTypeElement("undef.Undef"));
+            assertNotNull(task.getElements().getTypeElement("java.lang.String"));
+            assertNull(task.getElements().getTypeElement("undef2.Undef2"));
+            DeferredCompletionFailureHandler h = DeferredCompletionFailureHandler.instance(((JavacTaskImpl) task).getContext());
+            Field class2Flip = h.userCodeHandler.getClass().getDeclaredField("class2Flip");
+            class2Flip.setAccessible(true);
+            int size = ((Map) class2Flip.get(h.userCodeHandler)).size();
+            assertEquals(0, size);
+        }
+    }
+
+    private static void assertEquals(Object expected, Object actual) {
+        if (!Objects.equals(expected, actual)) {
+            throw new AssertionError("Unexpected value, expected: " + expected + ", actual: " + actual);
+        }
+    }
+
+    private static void assertNotNull(Object obj) {
+        if (obj == null) {
+            throw new AssertionError("Unexpected value, object: " + obj);
+        }
+    }
+
+    private static void assertNull(Object obj) {
+        if (obj != null) {
+            throw new AssertionError("Unexpected value, object: " + obj);
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        SymbolsDontCumulate t = new SymbolsDontCumulate();
+        t.testSymbolsCumulate();
+    }
+
+}