On Wed, 3 Nov 2021 09:50:08 GMT, Robbin Ehn <r...@openjdk.org> wrote:
>> src/hotspot/share/runtime/handshake.cpp line 350:
>>
>>> 348: }
>>> 349:
>>> 350: void Handshake::execute(HandshakeClosure* hs_cl, ThreadsListHandle*
>>> tlh_p, JavaThread* target) {
>>
>> Nit: can we drop the `_p` part of `tlh_p` please.
>
> Yes, please.
Fixed in handshake.[ch]pp.
>> src/hotspot/share/runtime/thread.cpp line 1764:
>>
>>> 1762: guarantee(Thread::is_JavaThread_protected(this, /* checkTLHOnly */
>>> true),
>>> 1763: "missing ThreadsListHandle in calling context.");
>>> 1764: if (is_exiting()) {
>>
>> Can't we remove this the same as we did for `java_suspend()`?
>
> Yes, please
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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4677
