Skip to content

Commit

Permalink
8325255: jdk.internal.util.ReferencedKeySet::add using wrong test
Browse files Browse the repository at this point in the history
Reviewed-by: rriggs, liach
  • Loading branch information
Jim Laskey committed Mar 6, 2024
1 parent 2bdd387 commit a7461de
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -439,4 +439,30 @@ private static <T> T internKey(ReferencedKeyMap<T, ReferenceKey<T>> 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 <T> type of key
*
* @return true if the key was added
*/
static <T> boolean internAddKey(ReferencedKeyMap<T, ReferenceKey<T>> setMap, T key) {
ReferenceKey<T> entryKey = setMap.entryKey(key);
setMap.removeStaleReferences();
ReferenceKey<T> 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;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public boolean contains(Object o) {

@Override
public boolean add(T e) {
return intern(e) == null;
return ReferencedKeyMap.internAddKey(map, e);
}

@Override
Expand Down
6 changes: 6 additions & 0 deletions test/jdk/jdk/internal/util/ReferencedKeyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ static void methods(ReferencedKeySet<Long> set) {
assertTrue(element1 == intern1, "intern failed"); // must be same object
assertTrue(intern2 != null, "intern failed");
assertTrue(element3 == intern3, "intern failed");

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 added after second attempt");
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
Expand Down

3 comments on commit a7461de

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shipilev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk22u

@openjdk
Copy link

@openjdk openjdk bot commented on a7461de May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shipilev the backport was successfully created on the branch backport-shipilev-a7461de2 in my personal fork of openjdk/jdk22u. To create a pull request with this backport targeting openjdk/jdk22u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a7461de2 from the openjdk/jdk repository.

The commit being backported was authored by Jim Laskey on 6 Mar 2024 and was reviewed by Roger Riggs and Chen Liang.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk22u:

$ git fetch https://github.com/openjdk-bots/jdk22u.git backport-shipilev-a7461de2:backport-shipilev-a7461de2
$ git checkout backport-shipilev-a7461de2
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk22u.git backport-shipilev-a7461de2

Please sign in to comment.