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

[Patricio Chilano Mateo]

On Fri, 4 Jun 2021 08:44:54 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> I still think we can keep the previous memory_order_release order, but if this stricter order makes it easier to read for others then I'm okay.
>
> 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.

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

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