RFR: 8257831: Suspend with handshakes [v3]

Robbin Ehn rehn at openjdk.java.net
Wed Apr 7 13:24:07 UTC 2021


On Tue, 6 Apr 2021 18:37:38 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>> 
>>  - Merge branch 'master' into SuspendInHandshake
>>  - Merge branch 'master' into SuspendInHandshake
>>  - 8257831: Suspend with handshake (review baseline)
>
> 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.

> src/hotspot/share/runtime/thread.hpp line 1142:
> 
>> 1140:   bool java_resume();  // higher-level resume logic called by the public APIs
>> 1141:   bool is_suspended()                 { return _handshake.is_suspended(); }
>> 1142:   bool suspend_request_pending()      { return _handshake.suspend_request_pending(); }
> 
> If you use is_suspended() to check for suspension in objectMonitor::enter() then we can remove this method.

Fixed

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

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


More information about the serviceability-dev mailing list