On Thu, 4 Nov 2021 01:18:23 GMT, David Holmes <[email protected]> wrote:
>> The rationale for removing the is_exiting() check from `java_suspend()` was
>> that it
>> was redundant because the handshake code detected and handled the
>> `is_exiting()`
>> case so we didn't need to do that work twice.
>>
>> If we look at `HandshakeState::resume()` there is no logic for detecting or
>> handling
>> the possibility of an exiting thread. That being said, we have to look
>> closer at what
>> `HandshakeState::resume()` does and whether that logic can be harmful if
>> executed
>> on an exiting thread.
>>
>> Here's the code:
>>
>> bool HandshakeState::resume() {
>> if (!is_suspended()) {
>> return false;
>> }
>> MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);
>> if (!is_suspended()) {
>> assert(!_handshakee->is_suspended(), "cannot be suspended without a
>> suspend request");
>> return false;
>> }
>> // Resume the thread.
>> set_suspended(false);
>> _lock.notify();
>> return true;
>> }
>>
>>
>> I'm not seeing anything in `HandshakeState::resume()` that
>> worries me with respect to an exiting thread. Of course, the
>> proof is in the testing so I'll rerun the usual testing after
>> deleting that code.
>
> A suspended thread cannot be exiting - else the suspend logic is broken. So,
> given you can call `resume()` on a not-suspended thread, as long as the
> handshake code checks for `is_supended()` (which it does) then no explicit
> `is_exiting` check is needed.
Agreed! I have to keep reminding myself that with handshake based suspend
and resume, we just don't have the same races with exiting threads that we
used to have.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4677
