On Sat, 10 Apr 2021 07:38:37 GMT, Richard Reingruber <rr...@openjdk.org> wrote:
>> `HandshakeState::suspend()` is a synchronous handshake and adding the >> handshake to the queue is lock free, but the execution of the synchronous >> handshake itself requires a `HandshakeState::claim_handshake()` call which >> does acquire the lock in question. We (the suspend requester) hold the lock >> while the handshake is being processed so we either detect that >> _handshakee->set_exiting() won the race (in the target thread) or we (the >> suspend requester) win the race of setting the suspend flag so the target >> thread can't exit yet. >> >> Hopefully that helps explain this dance. > > Hi Dan, > > thanks for picking up my question! > >> `HandshakeState::suspend()` is a synchronous handshake and adding the >> handshake to the queue is lock free, but the execution of the synchronous >> handshake itself requires a `HandshakeState::claim_handshake()` call which >> does acquire the lock in question. > > My point would be that the attempt to execute the synchronous handshake for > suspending a thread that is just about to call HandshakeState::thread_exit() > cannot make progress (blocks) while the target thread is not safe > (_thread_in_vm). > > A synchronous handshake requires the target thread to be in a safe state for > the > requester to execute the handshake operation. When executing > HandshakeState::thread_exit() the suspendee is _thread_in_vm. And the > requester > will find it to be `_not_safe` when calling `possibly_can_process_handshake()` > before calling `HandshakeState::claim_handshake()` or when calling > `can_process_handshake()` afterwards. In both cases try_process() returns with > failure _not_safe and the lock is not held. > > ++ > 546 if (!possibly_can_process_handshake()) { > 547 // JT is observed in an unsafe state, it must notice the handshake > itself > 548 return HandshakeState::_not_safe; > 549 } > 550 > 551 // Claim the mutex if there still an operation to be executed. > 552 if (!claim_handshake()) { > 553 return HandshakeState::_claim_failed; > 554 } > 555 > 556 // If we own the mutex at this point and while owning the mutex we > 557 // can observe a safe state the thread cannot possibly continue > without > 558 // getting caught by the mutex. > 559 if (!can_process_handshake()) { > 560 _lock.unlock(); > 561 return HandshakeState::_not_safe; > 562 } > > So isn't being unsafe sufficient to sync with suspend requests? Interesting point that I didn't pick up from your previous comment. Thanks for making it more clear for me. I need to mull on it for a bit. ------------- PR: https://git.openjdk.java.net/jdk/pull/3191