On Fri, 15 Oct 2021 22:19:15 GMT, Daniel D. Daugherty <dcu...@openjdk.org>
wrote:
>> So suspend_thread and resume thread's caller already takes a
>> ThreadsListHandle so this is unnecessary and never happens.
>
>> This seems an unrelated change in behaviour ??
>
> Actually this is equivalent code. The baseline code does this:
>
> ThreadsListHandle tlh;
> if (!tlh.includes(this)) {
> log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on
> ThreadsList, no suspension", p2i(this));
> return false;
> }
>
>
> What that code means is: if `this` thread does not appear in the newly created
> ThreadsListHandle's list, then `this` thread has called `java_suspend()` after
> `this` thread has exited. That's the only way that `this` thread could be
> missing
> from a newly created ThreadsListHandle's list.
>
> All I've done here is get rid of the ThreadsListHandle, verify that the
> calling
> context is protecting `this` and switch to an `is_exiting()` call.
>
>> So suspend_thread and resume thread's caller already takes a
>> ThreadsListHandle
>> so this is unnecessary and never happens.
>
> I presume @coleenp is saying that the `is_exiting()` check is not necessary.
>
> That might be true, it might not be true. I was trying to create equivalent
> code without creating a nested ThreadsListHandle here and I think I've
> done that. I actually think it might be possible for a JVM/TI event handler
> to fire after the thread has set is_exiting() and for that event handler to
> call SuspendThread() which could get us to this point.
On rereading all of these comments and the current baseline code, I have
to clarify one thing:
There is a minor change in behavior caused by switching from a
`ThreadsListHandle::includes()` check to a `JavaThread::is_exiting()`
check. The `JavaThread::is_exiting()` will return true before the target
JavaThread is removed from the system's current ThreadsList. So the
`JavaThread::is_exiting()` check will return true faster and the
`ThreadsListHandle::includes()` check.
However, that change in behavior does not result in a change in
behavior for a suspend thread request. Here's the relevant code:
src/hotspot/share/runtime/thread.cpp:
void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
<snip>
// The careful dance between thread suspension and exit is handled here.
// Since we are in thread_in_vm state and suspension is done with
handshakes,
// we can just put in the exiting state and it will be correctly handled.
set_terminated(_thread_exiting);
<snip>
// Remove from list of active threads list, and notify VM thread if we are
the last non-daemon thread
Threads::remove(this, daemon);
<snip>
}
When we changed the suspend thread mechanism to use handshakes, we
made it so that once the JavaThread called `set_terminated(_thread_exiting)`
it no longer had to honor a suspend thread request.
Summary: Yes, the `is_exiting()` call will result in detecting the exiting
JavaThread sooner than the `ThreadsListHandle::includes()` check.
However, it will not result in a change in suspend thread behavior.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4677
