On Tue, 6 Apr 2021 17:27:15 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>>> Also when the suspend request happened while A was blocked then after >>> current->set_thread_state_fence(_thread_blocked_trans); the check of >>> is_suspended() will return true even if the handshake state lock is not >>> acquired for the check >> >> That statement of mine is wrong. (Hopefully) correct is that after the >> complete >> state change from _thread_blocked to _thread_in_vm which includes being >> blocked >> for a safepoint/handshake the current thread would be able to check >> is_suspended() without holding the handshake state lock. It does not make a >> lot >> of sense then because in an unsafe state >> JavaThread::current()->is_suspended() >> will always return false. >> >>> 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. >> >> I understand that example now too: OM and suspend operations are unrelated. >> So I >> thought it would be ok for A to enter OM X, but it is not. A thread must not >> leave a safe state if it was suspended in that state (with a handshake). If >> it >> did, e.g., its stack could become not walkable. And it must not enter the >> monitor either. >> >> Sorry for the confusion. The check is good. > > But I still think we can just call is_suspended() here. If is_suspended() > returns false, then Z hasn't finished suspending A yet (either async > operation was still not added to the queue in case this is the first suspend > for A, or do_handshake() hasn't yet finished for that SuspendThreadHandshake > operation in case there was already an async operation in the queue). Since > we have already acquired OM, then Z will not see that after being suspended A > switched from not owning OM to owning it. It will always see that A owned OM. > And A will still block for the suspension in the 'else' clause since poll is > armed due to SuspendThreadHandshake operation(even if async operation was > still not added to the queue at the time A called is_suspended()), but we > wouldn't need to release OM. > If is_suspended() returns true then at most we will have a false positive, > since it could be that after the check somebody already resumed A, but that > can also happen even with suspend_request_pending() right after releasing > HandshakeState::_lock. > Is that correct or am I missing something? I'm not saying it doesn't work, I'm saying it works by accident (I never intended to be able to read the flag without HandshakeState lock). So yes, if the flag is false (lockless read), it is not possible for the suspend to have happened before we locked the OM. And once poll is armed for the synchrone handshake it will stay armed until after we executed the asynchrone handshake trap. What is possible is that we have armed for the suspend but not yet set the flag. If we consider the flag as the tipping point and not the start of execution of the suspend sync handshake, we can do the lockless read. ------------- PR: https://git.openjdk.java.net/jdk/pull/3191
