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

[Robbin Ehn]

On Mon, 24 May 2021 10:15:04 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> src/hotspot/share/runtime/handshake.cpp line 473:
>> 
>>> 471:   while (has_operation()) {
>>> 472:     MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
>>> 473:     HandshakeOperation* op = get_op_for_self();
>> 
>> I still don't understand why we now do a peek here, and then do the pop below (in remove-op), instead of just doing the pop here?
>
> The reason is, from above:
>>When poll is armed the handshakee must go to slow path and lock the HandshakeState lock if:
>>- 1: There is handshake operation to be processed
>>- 2: Some-else is in the middle of processing a handshake operation for this handshakee
> 
> This, in practice, means the first pointer of the queue most be non-NULL, and seen as non-NULL, until after we have processed the handshake operation.
> By doing:
> 1: get op
> 2: do op
>   - When handshakee first checks poll, it is armed.
>   - Then it checks global poll (safepoint) and the queue state.
>   -  If there is a handshake operation to be processed goto slow path.
> 3: remove op
>   - First ptr can be set NULL, which means the handshakee may elide the slow path.
>   - The handshakee sees poll armed, but no global poll and nothing in queue.
>   - The handshakee may disarm the poll.
> 
> By simply removing the OP after we have executed we know the handshakee must go to slow path.

Oh, sorry you mean when you are self processing?

It's because of symmetry, everyone do the same, which is far easier to think about.

>> src/hotspot/share/runtime/handshake.cpp line 570:
>> 
>>> 568:   Thread* current_thread = Thread::current();
>>> 569: 
>>> 570:   HandshakeOperation* op = get_op();
>> 
>> Again why not pop here?
>
> See above.
> 
> If there was only one OP in queue, the handshake may now disarm it's poll and continue.
> If there is more in queue it will still go to slow path.

So comment above applies here, we must not let queue become empty until OP is completed.

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

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