RFR: 8355065: ConcurrentModificationException in RichDiagnosticFormatter

[Liam Miller-Cushon]

On Sun, 20 Apr 2025 02:05:19 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Hi, please consider this fix for [JDK-8355065](https://bugs.openjdk.org/browse/JDK-8355065). RichDiagnosticFormatter is comparing type variables by their `toString` representation, and in this example the comparison fails due to type annotations. Replacing a call to `stripMetadataIfNeeded` with `stripMetadata` allows the comparison to succeed, and is consistent with the approach used in other parts of RichDiagnosticFormatter (e.g. for [the fix for JDK-8144580](https://github.com/openjdk/jdk/commit/570883e4dc5f769e17b3fcd8f2bbeb369dfe3a5d)).
>
> Hmm, now by stripping, now the metadata is no longer processed and the missing enum problem is no longer reported - this seems to just make the root CME more latent and eliminates one location where it can surface.

Thanks @liach for taking a look.

I agree `stripMetadataIfNeeded` seems like the wrong choice here, even if there are other opportunities to harden RichDiagnosticFormatter against issues like this.

> now the metadata is no longer processed and the missing enum problem is no longer reported

I think that's consistent with other errors missing classes, symbols are completed lazily and the errors only get reported if a missing symbol is noticed. The only place this annotation was being completed was unnecessarily during diagnostic formatting, by the call to `toString` on the unstripped type.

> this seems to just make the root CME more latent and eliminates one location where it can surface

The assumption in RichDiagnosticFormatter seems to be that it isn't reentrant, and it only process one diagnostic at a time. If `format` is called recursively the mutable state in `whereClauses` gets updated for the new diagnostic and not restored.

It might also be worthwhile to either update RichDiagnosticFormatter to detect if `format` is called recursively and report an error, or else make it safe and keep a stack of `whereClauses` state to make that correct.

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

PR Comment: https://git.openjdk.org/jdk/pull/24769#issuecomment-2818926384