On Tue, 6 Apr 2021 11:28:26 GMT, Richard Reingruber <rr...@openjdk.org> wrote:
>> To clarify, is_suspended() can only be checked when the JavaThread is >> _unsafe_, *after* you have transitioned from the safe state. >> 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. > >> >> >> 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/3191