JDK-8209055 : c.s.t.javac.code.DeferredCompletionFailureHandler seems to use WeakHashMap incorrectly
  • Type: Bug
  • Component: tools
  • Sub-Component: javac
  • Affected Version: 11.0.2,12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-08-07
  • Updated: 2022-02-14
  • Resolved: 2018-11-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 12
11.0.2Fixed 12 b20Fixed
Description
I've observed OOM from javac, where a half of the heap was held by com.sun.tools.javac.code.DeferredCompletionFailureHandler instances.

DeferredCompletionFailureHandler$Handler has WeakHashMap<ClassSymbol, FlipSymbolDescription> class2Flip field, where FlipSymbolDescription has a strong reference to ClassSymbol, which is always the same as a key. j.u.WHM guarantees that keys are weakly referenced, but doesn't give any guarantees on how values are referenced, in fact, there is an implementation note which states that values are held by strong references.
Comments
Fix Request Backporting this issue avoids OOM in javac, which improves resiliency. Patch applies cleanly to 11u, but test needs adjustments. 11u RFR: https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2019-February/000660.html
27-02-2019
URL: http://hg.openjdk.java.net/jdk/jdk/rev/1a6aeca4a8c9 User: jlahoda Date: 2018-11-12 09:43:36 +0000
12-11-2018
[~iignatyev], thanks a lot for testing!
30-10-2018
[~jlahoda], I've verified that your patch fixes the OOME I observed.
29-10-2018
Unfortunately, I can't reproduce original OOME w/ the latest JDK and error-prone; will try to find state when it was reproducible and check if your patch helps.
29-10-2018
I don't seem to be able to reproduce, but could you please try with this patch: https://bugs.openjdk.java.net/secure/attachment/79286/8209055 Thanks!
29-10-2018
I have seen OOME from a compilation of java.base w/ error-prone enabled. it requires one quite dirty hack in our makefiles[1] (you will need to update path to error_prone_ant.jar) and a hack in error-prone as well -- https://github.com/iignatev/error-prone/commit/82d57300d67f0b711883bc27d120efb31534b559 for the record, I don't see OOME nor NPE w/ suggested fix v2. [*] # boot jdk to generate tools that need to be run with the boot jdk. @@ -70,10 +71,27 @@ # The generate new bytecode javac setup uses the new compiler to compile for the # new jdk. This new bytecode might only be possible to run using the new jvm. $(eval $(call SetupJavaCompiler,GENERATE_JDKBYTECODE, \ - JVM := $(JAVA_JAVAC), \ + JVM := $(JAVA_JAVAC) \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.api=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.util=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.tree=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.main=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.code=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.processing=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.parser=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.comp=ALL-UNNAMED \ + --add-exports jdk.compiler.interim/com.sun.tools.javac.code=ALL-UNNAMED \ + --add-opens jdk.compiler.interim/com.sun.tools.javac.comp=ALL-UNNAMED \ + --add-opens jdk.compiler.interim/com.sun.tools.javac.code=ALL-UNNAMED \ + --add-modules java.logging \ + -cp /Users/iignatye/ws/error-prone/ant/target/error_prone_ant-2.3.2-SNAPSHOT.jar \ + -XX:+HeapDumpOnOutOfMemoryError, \ JAVAC := $(NEW_JAVAC), \ - FLAGS := -source 12 -target 12 --doclint-format html5 \ - -encoding ascii -XDignore.symbol.file=true $(JAVAC_WARNINGS), \ + FLAGS := -source 12 -target 12 --doclint-format html5 -XDcompilePolicy=simple \ + -encoding ascii -XDignore.symbol.file=true $(JAVAC_WARNINGS) \ + -Xmaxwarns -1 \ + -Xplugin:"ErrorProne -XepAllErrorsAsWarnings -XepExcludedPaths:.*/gensrc/.* -Xep:MissingOverride:OFF", \ + DISABLE_SJAVAC := true, \
07-08-2018
Ooops. Is there a way to reproduce, so that we can test if any patch works? Sadly, I don't think we can make type and members weak, as the FlipSymbolDescription is (I think) the only reference to them.
07-08-2018
suggested fix v2: http://cr.openjdk.java.net/~iignatyev//8209055/webrev.00/index.html wraps FSD::type, sym and members w/ WeakReference as those guys might have a strong reference to ClassSymbol.
07-08-2018
it appears that Type (FlipSymbolDescription::type) also has a strong reference to ClassSymbol, so the fix above is incomplete.
07-08-2018
on a second thought, the fix suggested above isn't good enough as w/ it there is a possibility to get NPE at 2nd line of flip method when sym become unreachable while we iterate thru class2Flip values. the proper fix should make sure sym is strongly reachable at the time we work on FlipSymbolDescription, e.g. diff -r 932efe020b20 src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.jav ... public void install() { - class2Flip.values().forEach(f -> f.flip()); + // must use forEach to be sure that corresponding ClassSymbol is reachable + class2Flip.forEach((k, v) -> v.flip()); } ... public void uninstall() { - class2Flip.values().forEach(f -> f.flip()); + // must use forEach to be sure that corresponding ClassSymbol is reachable + class2Flip.forEach((k, v) -> v.flip()); } }; ...
07-08-2018
suggested fix: diff -r 932efe020b20 src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java Mon Aug 06 15:35:44 2018 -0700 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/DeferredCompletionFailureHandler.java Tue Aug 07 00:45:20 2018 -0700 @@ -24,6 +24,7 @@ */ package com.sun.tools.javac.code; +import java.lang.ref.WeakReference; import java.util.Map; import java.util.WeakHashMap; @@ -144,14 +145,14 @@ } private static class FlipSymbolDescription { - public final ClassSymbol sym; + public final WeakReference<ClassSymbol> ref; public Type type; public Kind kind; public WriteableScope members; public Completer completer; public FlipSymbolDescription(ClassSymbol sym, Completer completer) { - this.sym = sym; + this.ref = new WeakReference<>(sym); this.type = sym.type; this.kind = sym.kind; this.members = null; @@ -159,6 +160,7 @@ } public void flip() { + ClassSymbol sym = ref.get(); Type prevType = sym.type; sym.type = type; this.type = prevType;
07-08-2018