On Wed, 25 Jun 2025 13:02:03 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> ThreadDumper/ThreadSnapshot need to handle a failure to resolve the native
>> VM JavaThread from a java.lang.Thread. This is hard to reproduce but a
>> thread that has since terminated can provoke a crash. Recognise this and
>> return a null ThreadSnapshot.
>
> Kevin Walls has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - comment update
> - comment update
Changes requested by dcubed (Reviewer).
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*`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/25958#pullrequestreview-2959809320
PR Review Comment: https://git.openjdk.org/jdk/pull/25958#discussion_r2167723492
