[lworld] RFR: 8372515: [lworld] Plumb in javac flags for compiling with preview mode [v2]

Jan Lahoda jlahoda at openjdk.org
Mon Dec 15 16:35:31 UTC 2025


On Mon, 15 Dec 2025 16:00:30 GMT, David Beaumont <duke at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java line 151:
>> 
>>> 149: 
>>> 150:     private void addCloseable(Closeable c) {
>>> 151:         synchronized (closeables) {
>> 
>> Here and elsewhere -- I'm not sure about `synchronized`. In general, javac operates under the assumption of "single threaded-ness" -- but I see around the file manager code that `synchronized` seems to be used. I'll leave this to @lahodaj
>
> I think in the compiler you can't assume single threaded-ness, and unless this adds a deadlock risk (which I am pretty sure it doesn't) it's definitely my preference to have correctness and avoid weird bugs/failures.

One instance of javac (i.e. one result of `ToolProvider.getSystemJavaCompiler().getTask(...)` is single-threaded, and cannot even be passed to a different thread without an external synchronization. There's no need for synchronization inside single instance of javac.

The case of file managers is a bit different: some backend data structures (like `JRTIndex` as such) are shared across various instances of file managers, and those definitely need to be synchronized. To what degree synchronization is needed here is unclear to me. The javadoc says that multi-threaded access to file managers is not required, and I don't think that concurrent use of the `JavacFileManager` from multiple instances of javac  is quite realistic (settings like multi-release or preview setting are kept in fields in the file manager, and the different instances of javac would talk past each other).

I.e. I don't think the synchronization here is really necessary. OTOH, the `JavacFileManager.getJRTIndex()` is currently synchronized, and it would seem more consistent for the `getJRTIndex()` and `closeables` to share the same synchronization approach. I would agree with Maurizio that I would mildly prefer to drop the synchronization, but I can live with the synchronization.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2620083325


More information about the valhalla-dev mailing list