RFR: 8257831: Suspend with handshakes [v2]

Robbin Ehn rehn at openjdk.java.net
Wed Apr 7 13:43:00 UTC 2021


On Wed, 31 Mar 2021 06:46:00 GMT, David Holmes <dholmes 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 two commits:
>> 
>>  - Merge branch 'master' into SuspendInHandshake
>>  - 8257831: Suspend with handshake (review baseline)
>
> src/hotspot/share/runtime/thread.hpp line 1146:
> 
>> 1144:   // if external|deopt suspend is present.
>> 1145:   bool is_suspend_after_native() const {
>> 1146:     return (_suspend_flags & (_obj_deopt JFR_ONLY(| _trace_flag))) != 0;
> 
> Comment at line 1144 needs updating.

Fixed

> src/hotspot/share/runtime/thread.inline.hpp line 194:
> 
>> 192: 
>> 193: inline void JavaThread::set_exiting() {
>> 194:   // use release-store so the setting of _terminated is seen more quickly
> 
> Pre-existing: A release doesn't push changes to main memory more quickly it only establishes ordering with previous loads and stores. Why do we need release semantics here - what state are with ordering with? What load-acquire are we pairing with?

I beilive this is legacy from pre-ThreadsList era.
Now we _should_ not have any such situations where such race matters.
A thread on a ThreadsList can become terminated but it is still perfectly reachable.
If we filter out terminated threads from a ThreadsList, any of those can become terminated just after any ways.

So Atomic::store/load is fine here IMHO.

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

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


More information about the serviceability-dev mailing list