On Wed, 7 Apr 2021 13:17:40 GMT, Robbin Ehn <r...@openjdk.org> wrote:
>> src/hotspot/share/runtime/handshake.hpp line 122:
>>
>>> 120: // must take slow path, process_by_self(), if _lock is held.
>>> 121: bool should_process() {
>>> 122: return !_queue.is_empty() || _lock.is_locked();
>>
>> While trying to think of the objectMonitor case I realize the order of this
>> should probably be reversed with a load fence in between. Otherwise we could
>> have the following scenario:
>>
>> Thread A blocks in ~ThreadInVMfromJava() -> wait_for_object_deoptimization()
>> with _thread_blocked state.
>> Thread Z executes SuspendThreadHandshake on A. Gets context switched before
>> adding the async handshake.
>> Thread A unblocks and transitions back to _thread_in_java. Calls
>> SafepointMechanism::process_if_requested()-> process_if_requested_slow()
>> since poll is armed. Checks that the handshake queue is empty. Gets context
>> switched.
>> Thread Z installs async handshake in queue, releases _lock and continues.
>> Thread A checks _lock is unlocked so it doesn't call
>> HandshakeState::process_by_self() to suspend and continues back to Java.
>> (Poll will still be armed though because we will see it in
>> update_poll_values()).
>
> You are correct.
Fixed, good catch!
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
