On Wed, 10 Dec 2025 11:09:17 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove note about StableValue (not possible)
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/file/JRTIndex.java line
> 139:
>
>> 137: /** Close underlying shared resources once no users exist
>> (called exactly once). */
>> 138: private synchronized void close() throws IOException {
>> 139: assert !isClosed;
>
> This should also use `Assert.check(isClosed)`.
> In fact, perhaps it might be worth pulling this check out in some
> `ensureOpen` method, and call that method whenever you need.
There's only really one other place, but (see my other comment about asserts) I
don't think the other case should be an assertion, since callers can just try
to use the instance after it's closed. Here, it's a bug *in this source file*
if close() were called twice (hence okay as an assert).
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1761#discussion_r2622824109
