Although its name might suggest otherwise, 
java.util.ConcurrentModificationException (CME) is not necessarily thrown in 
multithreaded context. For example, a map might throw CME if it detects 
modification in a midst of an operation which is supposed to be exclusive:

    jshell> Map<Integer, Integer> m = new HashMap<>();
       ...> m.computeIfAbsent(0, key -> m.put(1, 2));
    m ==> {}
    |  Exception java.util.ConcurrentModificationException
    |        at HashMap.computeIfAbsent (HashMap.java:1229)
    |        at (#2:1)

Somewhat similar happens in the reproducer: a computation of a tree path for a 
package element eventually modifies the very map that the result of that 
computation is to be put in. The map detects that and throws CME.

Here are more details. The tree path is to be found for the `bar` package 
element. The tree path is computed and put into the cache in one go 
(HashMap.computeIfAbsent). Usually that works fine, but not this time. Because 
the `bar` package resides in an implicitly referred module (test.bar), that 
package is discovered late. The computation triggers the `bar` package 
completion/parsing, which eventually calls back to JavadocEnter.visitTopLevel, 
which in turn puts an alternatively computed tree path for the `bar` package 
into that same cache. When the call stack returns to computeIfAbsent, that 
cache modification is detected and CME is thrown.

I can see three obvious ways of fixing this bug:

  1. Replace computeIfAbsent with an equivalent combination of simpler methods.
  2. Use a map whose computeIfAbsent can withstand non-exclusive modifications.
  3. Restructure the code so it avoids such modifications altogether.

(3) Is quite risky/complicated and not really needed at this time. (2) While 
it's unlikely to have noticeable performance overhead, ConcurrentHashMap feels 
like symptomatic treatment that is also out of place given that JavaDoc is 
single-threaded.

(1) seems the most reasonable. One potential problem with (1) though is that 
future refactoring may reintroduce the issue again, but in a way that the 
regression test won't catch.

-------------

Commit messages:
 - Initial commit

Changes: https://git.openjdk.org/jdk/pull/14310/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14310&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8304878
  Stats: 75 lines in 2 files changed: 74 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14310.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14310/head:pull/14310

PR: https://git.openjdk.org/jdk/pull/14310

Reply via email to