RFR: 8257831: Suspend with handshakes [v2]

[Robbin Ehn]

On Tue, 6 Apr 2021 11:49:42 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>>> 
>>> 
>>> 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 don't think that checking the suspended flag without holding the handshake
>> state lock would be too racy. Holding the OM is sufficient synchronization for
>> checking the suspended flag here.
>> 
>> If the suspender thread S did the suspend request while holding the OM then the
>> current thread T will see that it was suspended when it has entered the OM.
>> If S did the suspend without holding OM, only then the check would be racy but that would be ok.
>> 
>>> 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.
>
>> 
>> 
>> To clarify, is_suspended() can only be checked when the JavaThread is _unsafe_, _after_ you have transitioned from the safe state.
> 
> For a minute I misunderstood this and thought you meant this as a general rule
> for calling `is_suspended()` while there are examples where it is called without
> any synchronization (e.g. in `JvmtiEnv::GetThreadState()`) which can be ok.
> 
>> In this case we are checking if we are suspended before we have completed the transition because we need to know if we should drop the OM lock.
> 
> I think it is sufficient to hold the OM when calling `is_suspended` to decide if OM has to be dropped.

I do not follow, the OM is unrelated.
The suspender do not hold the OM.

What happens is:
- Thread A wait on OM X in blocked.
- Thread Z suspends Thread A, thread Z have no relation to OM X.
- Thread B unlocks OM X, thread B have no relation to the suspend request.
- Thread A locks OM X while blocked.
- Thread A was not allowed to lock OM X due to it is actually suspended.
- Thread A unlocks OM X and waits until resumed.

So while A and B fight over OM X, A is trying to figure out if Z suspended him while grabbing the lock.

The suspend flag and 'async handshake' flag are protected by the HandshakeState lock. (this is not lockless protocol)
The stores in SuspendThreadHandshake are not ordered, so they may happen in any order.
To read the stores there you must hold the HandshakeState lock.
Therefore if we want the exact answer to am A suspended we must read the flag under the lock.

Makes sense?

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

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