On Wed, 28 Jul 2021 10:03:10 GMT, David Holmes <dhol...@openjdk.org> wrote:
> If a thread is attaching via JNI and has not yet created its Thread object it
> can be caught in a ThreadSnapshot during a thread dump (VM_DumpThreads) of
> all threads**, and the threadObj() will be NULL, so we can't pass it to
> get_thread_status().
>
> ** JMM dumpThreads API
>
> The initial fix simply checks for a NULL threadObj() and reports thread
> status NEW in that case.
>
> Alternatively we could filter the attaching thread out completely in
> VM_DumpThreads::doit by expanding:
>
> if (jt->is_exiting() ||
> jt->is_hidden_from_external_view()) {
> // skip terminating threads and hidden threads
> continue;
> }
>
> to also check jt->is_attaching_via_jni().
>
> Note that higher-level code already filters out ThreadSnapshots with NULL
> threadObj() anyway so we could go either way.
>
> Testing: manual hacks - see bug report.
> - tier 1-3 sanity testing
>
> Thanks,
> David
Nice catch!
I'm always happy when we find a reason for one of these 24 hour
stress test failures.
Thumbs up! Your call on whether to change the code as I mentioned
in my other comment.
@sspitsyn - It would be great to have a Serviceability person chime in on this
review...
src/hotspot/share/services/threadService.cpp line 879:
> 877: // If thread is still attaching then threadObj will be NULL.
> 878: _thread_status = threadObj == NULL ? JavaThreadStatus::NEW
> 879: :
> java_lang_Thread::get_thread_status(threadObj);
I like the use of `JavaThreadStatus::NEW` here.
Since L867 creates the _threadObj OopHandle, do you want to change both
uses of `threadObj` to `_threadObj()`? Is that still how we fetch the oop from
an OopHandle? Or even use `threadObj()`... Then we don't have to reason
about whether the `threadObj` oop is still good...
-------------
Marked as reviewed by dcubed (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4921
