On Mon, 31 Jul 2023 13:34:30 GMT, Jim Laskey <jlas...@openjdk.org> wrote:
>> java.lang.runtime.ReferencedKeyMap was introduced to provide a concurrent >> caching scheme for Carrier objects. The technique used is generally useful >> for a variety of caching schemes and is being moved to be shared in other >> parts of the jdk. The MethodType interning case is one example. > > Jim Laskey has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 17 commits: > > - Merge branch 'master' into 8310913 > - Update implNote for intern > - Merge branch 'master' into 8310913 > - Requested changes. > > Added intern with UnaryOperator<T> interning function to prepare key > before adding to set. > - Update test to check for gc. > - Update ReferencedKeyTest.java > - Simple versions of create > - Add flag for reference queue type > - Merge branch 'master' into 8310913 > - Update to use VirtualThread friendly stale queue. > - ... and 7 more: https://git.openjdk.org/jdk/compare/408987e1...af95e5ae src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java line 151: > 149: @Override > 150: public boolean add(T e) { > 151: return intern(e) == null; This line is wrong, as `intern(…)` will never return `null`. -------------------------------------------------------------------------------- This is the closest to the correct implementation, Suggestion: return !contains(e) & intern(e) == e; but may incorrectly return `true` in case of the following data race, assuming `t1e == t2e`: 1. **Thread 1** calls `contains(t1e)` and gets `false`. 2. **Thread 2** calls `intern(t2e)` and successfully adds `t2e`. 3. **Thread 1** calls `intern(t1e)` and gets back `t2e`, which is `==` to `t1e`. 4. **Thread 1** returns `true`, even though it was **Thread 2** which modified the `ReferencedKeySet`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14684#discussion_r1477004059
