On Wed, 25 Jun 2025 23:50:59 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> src/hotspot/share/services/threadService.cpp line 1477:
>>
>>> 1475: java_thread = java_lang_Thread::thread(thread_h());
>>> 1476: if (java_thread == nullptr) {
>>> 1477: return nullptr; // thread terminated
>>
>> This is not the right way to determine if you have a valid JavaThread
>> when you have created a ThreadsListHandle. This code near the top
>> of `ThreadSnapshotFactory::get_thread_snapshot` is not right:
>>
>>
>> ThreadsListHandle tlh(THREAD);
>> ResourceMark rm(THREAD);
>> HandleMark hm(THREAD);
>> Handle thread_h(THREAD, JNIHandles::resolve(jthread));
>>
>>
>> The above code was added by:
>> [JDK-8357650](https://bugs.openjdk.org/browse/JDK-8357650) ThreadSnapshot to
>> take snapshot of thread for thread dumps
>>
>> Here's the example code from src/hotspot/share/runtime/threadSMR.hpp:
>>
>> // JNI jobject example:
>> // jobject jthread = ...;
>> // :
>> // ThreadsListHandle tlh;
>> // JavaThread* jt = nullptr;
>> // bool is_alive = tlh.cv_internal_thread_to_JavaThread(jthread, &jt,
>> nullptr);
>> // if (is_alive) {
>> // : // do stuff with 'jt'...
>> // }
>>
>>
>> So instead of this line:
>>
>> Handle thread_h(THREAD, JNIHandles::resolve(jthread));
>>
>> which does not guarantee you a valid JavaThread handle, you should
>> use `tlh.cv_internal_thread_to_JavaThread` to get a `JavaThread*`.
>
> Great catch Dan! I totally missed the TLH at the start of
> `get_thread_snapshot`. I knew something was off here but couldn't quite put
> my finger on it.
Yes thanks Dan! Will update.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25958#discussion_r2168498780
