Skip to content

Commit

Permalink
8271855: [TESTBUG] Wrong weakCompareAndSet assumption in UnsafeIntrin…
Browse files Browse the repository at this point in the history
…sicsTest

Reviewed-by: goetz, thartmann
  • Loading branch information
TheRealMDoerr committed Sep 29, 2021
1 parent 756d22c commit c4d1157
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions test/hotspot/jtreg/compiler/gcbarriers/UnsafeIntrinsicsTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -24,7 +24,7 @@
/*
* @test id=Z
* @key randomness
* @bug 8059022
* @bug 8059022 8271855
* @modules java.base/jdk.internal.misc:+open
* @summary Validate barriers after Unsafe getReference, CAS and swap (GetAndSet)
* @requires vm.gc.Z
Expand Down Expand Up @@ -301,15 +301,15 @@ private Node mergeImplSwap(Node startNode, Node expectedNext, Node head) {
private Node mergeImplCAS(Node startNode, Node expectedNext, Node head) {
// CAS - should always be true within a single thread - no other thread can have overwritten
if (!UNSAFE.compareAndSetReference(startNode, offset, expectedNext, head)) {
throw new Error("CAS should always succeed on thread local objects, check you barrier implementation");
throw new Error("CAS should always succeed on thread local objects, check your barrier implementation");
}
return expectedNext; // continue on old circle
}

private Node mergeImplCASFail(Node startNode, Node expectedNext, Node head) {
// Force a fail
if (UNSAFE.compareAndSetReference(startNode, offset, "fail", head)) {
throw new Error("This CAS should always fail, check you barrier implementation");
throw new Error("This CAS should always fail, check your barrier implementation");
}
if (startNode.next() != expectedNext) {
throw new Error("Shouldn't have changed");
Expand All @@ -318,17 +318,23 @@ private Node mergeImplCASFail(Node startNode, Node expectedNext, Node head) {
}

private Node mergeImplWeakCAS(Node startNode, Node expectedNext, Node head) {
// Weak CAS - should always be true within a single thread - no other thread can have overwritten
if (!UNSAFE.weakCompareAndSetReference(startNode, offset, expectedNext, head)) {
throw new Error("Weak CAS should always succeed on thread local objects, check you barrier implementation");
// Weak CAS - should almost always be true within a single thread - no other thread can have overwritten
// Spurious failures are allowed. So, we retry a couple of times on failure.
boolean ok = false;
for (int i = 0; i < 3; ++i) {
ok = UNSAFE.weakCompareAndSetReference(startNode, offset, expectedNext, head);
if (ok) break;
}
if (!ok) {
throw new Error("Weak CAS should almost always succeed on thread local objects, check your barrier implementation");
}
return expectedNext; // continue on old circle
}

private Node mergeImplWeakCASFail(Node startNode, Node expectedNext, Node head) {
// Force a fail
if (UNSAFE.weakCompareAndSetReference(startNode, offset, "fail", head)) {
throw new Error("This weak CAS should always fail, check you barrier implementation");
throw new Error("This weak CAS should always fail, check your barrier implementation");
}
if (startNode.next() != expectedNext) {
throw new Error("Shouldn't have changed");
Expand All @@ -340,7 +346,7 @@ private Node mergeImplCMPX(Node startNode, Node expectedNext, Node head) {
// CmpX - should always be true within a single thread - no other thread can have overwritten
Object res = UNSAFE.compareAndExchangeReference(startNode, offset, expectedNext, head);
if (!res.equals(expectedNext)) {
throw new Error("Fail CmpX should always succeed on thread local objects, check you barrier implementation");
throw new Error("Fail CmpX should always succeed on thread local objects, check your barrier implementation");
}
return expectedNext; // continue on old circle
}
Expand Down

3 comments on commit c4d1157

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@TheRealMDoerr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on c4d1157 Oct 15, 2021

Choose a reason for hiding this comment

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

@TheRealMDoerr @TheRealMDoerr the backport was successfully created on the branch TheRealMDoerr-backport-c4d11570 in my personal fork of openjdk/jdk17u. To create a pull request with this backport targeting openjdk/jdk17u: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 c4d11570 from the openjdk/jdk repository.

The commit being backported was authored by Martin Doerr on 29 Sep 2021 and was reviewed by Goetz Lindenmaier and Tobias Hartmann.

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/jdk17u:

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

Please sign in to comment.