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

[David Beaumont]

On Tue, 25 Nov 2025 14:52:23 GMT, David Beaumont <duke at openjdk.org> wrote:

> Plumbing for javac flags, mostly inspired by/copied from test commits made by @lahodaj .
> 
> There are several things here, mostly entangled, so it's a bit tricky to try splitting this out, but it would be possible if people wanted.
> 
> The biggest "refactoing" part of this PR is "src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java" which now has a properly controlled lifecycle and disposed its resources correctly. Prior to this, the class used a non-closeable JRT file-system reference, which leads to "persistent open file" issues such as JDK-8357249.
> 
> This *does* mean that if compilation and the runtime have the same preview mode, then a 2nd JRT file system to the same jimage file is "opened", *but* the file system itself is lightweight, non-caching and both of them will use the underlying SharedImageReader (which is where nodes are cached etc.) so it really shouldn't be an issue (I will make sure javac benchmarks are checked however).
> 
> The benefit of this is that now, the shared index (which does do some caching) is correctly tracked across all users, and will be closed when the last user closes the lightweight wrapper instance.
> 
> A lot of the smaller "spot fix" changes in this PR were just copied by me, or at least inspired directly by Jan's work, so I may have missed some semantic subtlety in the code I'm not familiar with. Please evaluate that carefully.

A couple of notes because some of this is subtle and I want reviewers to not miss some parts.

src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java line 221:

> 219:         } else {
> 220:             useCtProps = false;
> 221:         }

Much simpler to reason about lifetimes if the index (wrapper) is always closeable.
I've also made JRTIndex implement Closeable, which seemed sensible, but since it's a public class I want to be sure that's a permitted thing to do in this part of the codebase.

src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line 76:

> 74:     }
> 75: 
> 76:     private static class FileSystemResources {

Just moving all the outer class code into here. The actual changes are pretty small.
Mostly just the static management of the preview/non-preview versions and introduction of close semantics.

src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line 120:

> 118:             int idx = previewMode ? 1 : 0;
> 119:             boolean shouldClose;
> 120:             synchronized (FileSystemResources.class) {

Be careful to not call close() with the class synchronized.

src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line 317:

> 315:     public void close() throws IOException {
> 316:         // Release is atomic and succeeds at most once.
> 317:         if (!sharedResources.release(this)) {

This exception may be contentious. Please speak up if you don't like it.
It's here to track any accidental double-closing of the index, and did catch one case already.
Since this is an internal class it feels like we should be able to track and eliminate any double close events, but it is a runtime exception in complex code, so I'm happy to remove it or maybe replace it with logging of some kind.
Please speak up if you have opinions.

src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java line 449:

> 447:         @Override
> 448:         public void close() throws IOException {
> 449:             if (jrtIndex != null) {

I made sure it's safe to call close() more than once on the same instance, which saves having to null the reference here and worry about synchronization at this level.
However, I also understand if someone wanted to clear this reference so it could be GC'd.
Please speak up if you have an opinion.

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

PR Review: https://git.openjdk.org/valhalla/pull/1761#pullrequestreview-3505493360
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2560319288
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2560361334
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2560366129
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2560384754
PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2560350136