On Wed, 7 Apr 2021 07:25:29 GMT, Robbin Ehn <r...@openjdk.org> wrote:
>> src/hotspot/share/runtime/handshake.cpp line 463:
>>
>>> 461: ThreadInVMForHandshake tivm(_handshakee);
>>> 462: {
>>> 463: ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id());
>>
>> Why is this needed when it is inside ThreadInVMForHandshake constructor ??
>
> If we process the async suspension handshake we can go to safepoint.
> And before safepoint we must drop the tty lock.
Is this worth a comment above the `break_tty_lock_for_safepoint()` call?
>> src/hotspot/share/runtime/thread.cpp line 1442:
>>
>>> 1440:
>>> 1441: // The careful dance between thread suspension and exit is
>>> handled here:
>>> 1442: _handshake.thread_exit();
>>
>> I don't like the fact this hides the set_terminating call.
>>
>> In fact I think I'd rather keep this in-line rather than tucking it away
>> inside the handshake code. This looks too much like we're informing the
>> handshake that the thread is exiting rather then coordinating thread
>> termination with a late suspension request.
>
> The problem is that we need access to the private _lock to coordinate this.
Perhaps rename the function to something like handle_thread_exit_race()?
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
