On Mon, 5 Jun 2023 10:47:59 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
> 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.
This pull request has now been integrated.
Changeset: 01455a07
Author: Pavel Rappo <pra...@openjdk.org>
URL:
https://git.openjdk.org/jdk/commit/01455a07a7e1f15aed43cd47222047810c826abd
Stats: 77 lines in 2 files changed: 76 ins; 1 del; 0 mod
8304878: ConcurrentModificationException in javadoc tool
Reviewed-by: jjg
-------------
PR: https://git.openjdk.org/jdk/pull/14310
