On Tue, 25 Nov 2025 14:52:23 GMT, David Beaumont <[email protected]> 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.
src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line 147:
> 145:
> 146: synchronized Entry getEntry(RelativeDirectory rd) throws
> IOException {
> 147: if (isClosed) {
One other example of a new exception, not previously possible. This is rather
unavoidable post-closure, and feels like IOException is the appropriate
response (as opposed to the IllegalStateException proposed in close()).
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1761#discussion_r2560454520
