From d119fb971d7bb15c38a340a845048d437b1a440b Mon Sep 17 00:00:00 2001 From: JimLaskey Date: Tue, 6 Feb 2024 08:49:15 -0400 Subject: [PATCH 1/3] Correct test --- .../share/classes/jdk/internal/util/ReferencedKeySet.java | 2 +- test/jdk/jdk/internal/util/ReferencedKeyTest.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java index 807eea87dfe1c..f9e22952a1a07 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java @@ -148,7 +148,7 @@ public boolean contains(Object o) { @Override public boolean add(T e) { - return intern(e) == null; + return intern(e) == e; } @Override diff --git a/test/jdk/jdk/internal/util/ReferencedKeyTest.java b/test/jdk/jdk/internal/util/ReferencedKeyTest.java index c5edaedd2e25e..b272949997433 100644 --- a/test/jdk/jdk/internal/util/ReferencedKeyTest.java +++ b/test/jdk/jdk/internal/util/ReferencedKeyTest.java @@ -127,6 +127,11 @@ static void methods(ReferencedKeySet set) { assertTrue(element1 == intern1, "intern failed"); // must be same object assertTrue(intern2 != null, "intern failed"); assertTrue(element3 == intern3, "intern failed"); + + Long value1 = new Long(BASE_KEY + 999); + Long value2 = new Long(BASE_KEY + 999); + assertTrue(set.add(value1), "key not added"); + assertTrue(!set.add(value2), "key should not have been added"); } // Borrowed from jdk.test.lib.util.ForceGC but couldn't use from java.base/jdk.internal.util From 51cf3e4abd5e19a451d7471f00d27f84f98a7040 Mon Sep 17 00:00:00 2001 From: JimLaskey Date: Tue, 6 Feb 2024 14:38:51 -0400 Subject: [PATCH 2/3] Update ReferencedKeyTest.java --- test/jdk/jdk/internal/util/ReferencedKeyTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/jdk/jdk/internal/util/ReferencedKeyTest.java b/test/jdk/jdk/internal/util/ReferencedKeyTest.java index b272949997433..f3ba480f91b59 100644 --- a/test/jdk/jdk/internal/util/ReferencedKeyTest.java +++ b/test/jdk/jdk/internal/util/ReferencedKeyTest.java @@ -131,6 +131,7 @@ static void methods(ReferencedKeySet set) { Long value1 = new Long(BASE_KEY + 999); Long value2 = new Long(BASE_KEY + 999); assertTrue(set.add(value1), "key not added"); + assertTrue(set.add(value1), "key not added after second attempt"); assertTrue(!set.add(value2), "key should not have been added"); } From 728599199a3810218357d6b3066747ec12075f35 Mon Sep 17 00:00:00 2001 From: JimLaskey Date: Tue, 5 Mar 2024 16:20:56 -0400 Subject: [PATCH 3/3] Fix ReferencedKeySet::add --- .../jdk/internal/util/ReferencedKeyMap.java | 26 +++++++++++++++++++ .../jdk/internal/util/ReferencedKeySet.java | 2 +- .../jdk/internal/util/ReferencedKeyTest.java | 6 ++--- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java index f102e3c94e151..be392c3ae2ddf 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java @@ -439,4 +439,30 @@ private static T internKey(ReferencedKeyMap> setMap, T ke return interned; } + + /** + * Attempt to add key to map if absent. + * + * @param setMap {@link ReferencedKeyMap} where interning takes place + * @param key key to add + * + * @param type of key + * + * @return true if the key was added + */ + static boolean internAddKey(ReferencedKeyMap> setMap, T key) { + ReferenceKey entryKey = setMap.entryKey(key); + setMap.removeStaleReferences(); + ReferenceKey existing = setMap.map.putIfAbsent(entryKey, entryKey); + if (existing == null) { + return true; + } else { + // If {@code putIfAbsent} returns non-null then was actually a + // {@code replace} and older key was used. In that case the new + // key was not used and the reference marked stale. + entryKey.unused(); + return false; + } + } + } diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java index f9e22952a1a07..21b940439e00c 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java @@ -148,7 +148,7 @@ public boolean contains(Object o) { @Override public boolean add(T e) { - return intern(e) == e; + return ReferencedKeyMap.internAddKey(map, e); } @Override diff --git a/test/jdk/jdk/internal/util/ReferencedKeyTest.java b/test/jdk/jdk/internal/util/ReferencedKeyTest.java index f3ba480f91b59..75690fa6e258a 100644 --- a/test/jdk/jdk/internal/util/ReferencedKeyTest.java +++ b/test/jdk/jdk/internal/util/ReferencedKeyTest.java @@ -128,10 +128,10 @@ static void methods(ReferencedKeySet set) { assertTrue(intern2 != null, "intern failed"); assertTrue(element3 == intern3, "intern failed"); - Long value1 = new Long(BASE_KEY + 999); - Long value2 = new Long(BASE_KEY + 999); + Long value1 = Long.valueOf(BASE_KEY + 999); + Long value2 = Long.valueOf(BASE_KEY + 999); assertTrue(set.add(value1), "key not added"); - assertTrue(set.add(value1), "key not added after second attempt"); + assertTrue(!set.add(value1), "key added after second attempt"); assertTrue(!set.add(value2), "key should not have been added"); }