RFR: 8257831: Suspend with handshakes [v2]

[Robbin Ehn]

On Tue, 30 Mar 2021 13:43:09 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - Merge branch 'master' into SuspendInHandshake
>>  - 8257831: Suspend with handshake (review baseline)
>
> src/hotspot/share/runtime/objectMonitor.cpp line 415:
> 
>> 413:       current->set_thread_state_fence(_thread_blocked_trans);
>> 414:       if (SafepointMechanism::should_process(current) &&
>> 415:         current->suspend_request_pending()) {
> 
> Shouldn't `suspend_request_pending()` be replaced with `is_suspended()`? See also comment on declaration of `_suspend_requested`.
> And isn't checking `SafepointMechanism::should_process(current)` redundant?

We don't want to always to take a lock (handshake state lock), if the poll is disarmed there cannot be any suspend request.
We only need to unlock the OM for suspends that happened before we grabbed the OM lock in EnterI().

When we leave EnterI we are still blocked, this means a thread might be processing the suspend handshake, which was emitted before we did EnterI().
To synchronize the suspend check with such a thread we need to grab the HandshakeState lock, e.g. we cannot racy check the suspended flag.

I choosed to check for an async suspension handshake that needs to be processed. (We can have such without being suspended). We could ignored that async handshake by just checking is_suspended, it would be processed in the else statement instead with "SafepointMechanism::process_if_requested(current);" instead.

But we avoid releasing the OM lock in cases where we already have been resumed.

So I can change to is_suspended inside the the method, but we still must do it under the HandshakeState lock.

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

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