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

[Patricio Chilano Mateo]

On Thu, 3 Jun 2021 07:46:45 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Right, but I don't see why the change of just using the queue to check for pending handshakes matters here. This release was to ensure that if the handshakee executed the closure, the memory operations executed in the handshake would be visible to the vmthread/handshaker after seeing the operation was completed (matched with the acquired after is_completed()). What could be the issue if this is not a full fence?
>
> "Change of just using the queue to check for pending handshakes": also means we must remove the handshake from the queue after it have been executed.
> I consider the decrement as the marking of "it have been executed".
> 
> Therefore anything after the decrement, should stay after and not float above the dec, to follow the protocol.
> If disregard ordering side-effects when removing the last in queue with CAS, if the removal happens before the decrement means if the handshakee ask "should process" it could say no before we decrement.
> 
> This release to the removal of the operation is match with the acquire on is_empty().
> Normally the removal would had used the release from the mutex lock, but here we hold the lock over several operations.

Ok, although since all it matters for is_empty() is that there is at least one element in the queue and to remove the last element we always use a CAS it should be okay. I now realize that we could have also removed the operation from the queue as before if it is not the last one, so that would save the second traversal of the queue. But I see that if we want to maintain the invariant "If the handshake operation is not completed it must be on at least one queue" then we need to do it like this.

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

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