On Tue, 6 Apr 2021 12:23:06 GMT, Richard Reingruber <rr...@openjdk.org> wrote:

>> 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?
>
>> 
>> 
>> 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.
> 
> If understand the example correctly then the suspend operation and the
> operations on OM X are unordered with respect to each other. If so, then we 
> can
> put them into an order where the suspend happens after "Thread A locks OM X 
> while
> blocked.". We are free to do this; there's no synchronization that would 
> prevent it.
> 
> 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. And if Z tried to suspend A after the state change 
> then
> Z will block because A is not safe anymore.
> 
> Sorry for insisting. I really hope I'm not wrong ;)

The only reason _suspended is volatile is to be able to make the the fast check 
in resume().
So disregard that early check and that it is volatile, the users of the flag 
uses HandshakeState lock for synchronizing reads and stores to that flag.

E.g
set_suspend(true);
set_async_suspend_handshake(true);
log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended, arming 
ThreadSuspension", p2i(_handshakee));
ThreadSuspensionHandshake* ts = new ThreadSuspensionHandshake();
Handshake::execute(ts, _handshakee);

Before we have enqueued the async handshake there is no trap and but the flag 
set_suspend(true).
This means we will miss the async handshake and continue to executed with 
suspended flag.

Both flags and async handshake are set atomically by serializing over the 
HnadshakeState lock.

In this case we want to both know if we are suspended if so process the 
suspension.

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

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

Reply via email to