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

[Patricio Chilano Mateo]

On Wed, 2 Jun 2021 09:50:34 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> And does the comment on L326 now need to be updated?
>
> We have three options here:
> 1: "If the handshake operation is not completed it must be on at least one queue."
> 2: "If the handshake operation is completed it is not on any queue."
> 3: "It may be on queue while completed and may not be queue when not completed, it depends."
> 
> With the polling mechanism saying you must stop while the queue is not empty, the operation is removed after it is executed, option 1 seemed preferable to me.
> Specially if we want a simpler easier model to avoid any further such bugs as this.
> 
> This means removal from queue happens after decrement of _pending_threads.
> remove_op() -> pop() gives no ordering guarantee, those comes from the mutex.
> 
> The technical note on the current implementation of the queue is:
> Only first can be read without lock and the implementation of first manipulation is done with a CAS.
> So a side-effect of current implementation is that the only thing that can be seen will be ordered.
> 
> Since we running in slowpath there is no need to reduce ordering to a minimum and we can thus change the queue implementation since it have no guarantees to do any ordering without worrying about someone (ab)using side-effects.
> 
> Do you agree?

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?

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

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