On Wed, 10 Dec 2025 10:34:35 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 88: > >> 86: >> 87: // Synchronized by FileSystemResources.class, NOT instance. >> 88: private final Set<JRTIndex> owners = new HashSet<>(); > > This seems like a ref counter -- e.g. we want to keep track of how many > JRTIndex instances there are that point at the same underlying resources. > This seems sensible, and doing it this way seems better than using a > refcount, so that you can check that the guy who did the close is the same as > that who did the open. > > Since there's only one set for both modes, correctness currently hinges (I > think) on the fact that JRTIndex does *not* override equals/hashCode. This > means that, if the same client opens two indices, one for preview, one for > non-preview, you will see two distinct entries in the set (because the > comparison is an identity comparison). > > We might want to leave a comment about this "hidden" dependencies. Or, we > could split the owner set in two. If we do the latter, we might improve the > "already opened" check -- so that we can say "index already opened _in mode > XYZ_" Yes, it's "ref counting" but with non-fungeable counters. And yes, it implicitly relies on identity hashing (which should be made note of somewhere). Splitting the owner-set into two or having a different internal key would be an option, but I think there's no reason to expect a class like JRTIndex to ever need more than system identity semantics (it's a service provider not a data object), so a comment is probably good for now. I think JRTIndex can (and should) be made final, and I think a class-level comment should then suffice for future maintainers. Thanks for bringing this up. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2619965833
