RFR: 8266557: assert(SafepointMechanism::local_poll_armed(_handshakee)) failed: Must be [v3]

[Robbin Ehn]

On Fri, 4 Jun 2021 18:15:04 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Added comment.
>> 
>> But that would be relying on implementation details of a generic queue.
>> This would be a perfectly fine change-set:
>> 
>> diff --git a/src/hotspot/share/utilities/filterQueue.inline.hpp b/src/hotspot/share/utilities/filterQueue.inline.hpp
>> index 2fa199f2b04..ecb13e57db7 100644
>> --- a/src/hotspot/share/utilities/filterQueue.inline.hpp
>> +++ b/src/hotspot/share/utilities/filterQueue.inline.hpp
>> @@ -94 +94 @@ E FilterQueue<E>::pop(MATCH_FUNC& match_func) {
>> -      if (Atomic::cmpxchg(&_first, match, match->_next) == match) {
>> +      if (Atomic::cmpxchg(&_first, match, match->_next, memory_order_relaxed) == match) {
>> 
>> 
>> 
>> CAS will happened after we load _first, since it's loaded with acquire, but _both_ operation may float above decrement!
>> 
>> Now handshake will break because it would be using a undocumented side affect.
>> If we are removing any ordering, we should remove it where it is not needed. (the CAS)
>> Not the other way around!
>
> As long as we have a release() fence after the handshake closure the reordering of decrementing _pending_threads and removing the operation from the queue shouldn't matter since the target doesn't read _pending_threads.

"release operation: no reads or writes in the current thread can be reordered after this store."

But stores happened after this store may be reordered before. Which means the removal might happen even before the handshake operation was executed.

So we need to stop the store ("the relaxed CAS") from floating above the dec.

Dec with rel+acq gives: "No memory reads or writes in the current thread can be reordered before or after this store."

-------------

PR: https://git.openjdk.java.net/jdk/pull/3973