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

Maurizio Cimadamore mcimadamore at openjdk.org
Wed Dec 10 10:44:22 UTC 2025


On Tue, 25 Nov 2025 15:30:51 GMT, David Beaumont <duke at openjdk.org> wrote:

>> 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.
>
> Also, nulling out "sharedResources" during close() for better GC cleanup is doable, but not completely trivial, since it adds "post-closure" failure modes that didn't exist before.
> I do think that *somewhere* we should be detaching the index for garbage collection, but maybe all its users are sufficiently well scoped that it's unlikely to matter.

In general, I prefer to err on the side of correctness (as you did here). Given all allocation of JRTIndex go through the factory (and the shared file system resources), if we detect something suspicious it likely means there's an issue somewhere. Now, we could argue if this should be a crash or not -- but if it's not then we'll likely never know about it.

Note -- javac has a class to do some assertion checks -- see com.sun.tools.javac.util.Assert.

So this could also be rewritten as:


Assert.check(sharedResources.release(this), "Already closed");

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

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


More information about the valhalla-dev mailing list