RFR: 8364314: java_lang_Thread::get_thread_status fails assert(base != nullptr) failed: Invalid base [v2]

[Daniel D. Daugherty]

On Wed, 30 Jul 2025 06:24:39 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> After the changes in  JDK-8361912 we could "return " the carrier thread from `cv_internal_thread_to_JavaThread`, but before we hit the transition disabler the virtual thread could unmount. As a result when we execute this code:
>> 
>>   if (is_virtual) {
>>     // 1st need to disable mount/unmount transitions
>>     transition_disabler.init(jthread);
>> 
>>     carrier_thread = Handle(THREAD, java_lang_VirtualThread::carrier_thread(thread_h()));
>>     if (carrier_thread != nullptr) {
>>       java_thread = java_lang_Thread::thread(carrier_thread());
>>     }
>>   }
>> 
>> we hit the implicit else where "`carrier_thread == nullptr`" and we do nothing, but `java_thread` still holds the old carrier, which we then perform the handshake operation with:
>> 
>>   void do_thread(Thread* th) override {
>>     Thread* current = Thread::current();
>> 
>>     bool is_virtual = java_lang_VirtualThread::is_instance(_thread_h());
>>     if (_java_thread != nullptr) {
>>       if (is_virtual) {
>>         // mounted vthread, use carrier thread state
>>         oop carrier_thread = java_lang_VirtualThread::carrier_thread(_thread_h());
>>         _thread_status = java_lang_Thread::get_thread_status(carrier_thread);
>>       } else {
>> 
>> But the `_java_thread` no longer has a carrier, so `get_thread_status` is passed null and we crash.
>> 
>> Simple fix is to clear `java_thread` when we find a null carrier oop. Also added an assert to guard against a null carrier oop in the handshake code, and added some additional commentary.
>> 
>> Testing:
>>  - com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java
>>  -  tier 5 and 6
>> 
>> Thanks
>
> David Holmes has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Remove from ProblemList
>  - Merge branch 'master' into 8364314-threadSMR
>  - 8364314: java_lang_Thread::get_thread_status fails assert(base != nullptr) failed: Invalid base

Thumbs up. I have a single typo and a suggested rewording for a comment.

src/hotspot/share/services/threadService.cpp line 1483:

> 1481:       // Note: this java_thread may not be protected by the ThreadsListHandle above,
> 1482:       // but as we have disabled transitions, if we are mounted on it, then it can
> 1483:       // not terminate and so is safe to handshake with.

Perhaps:

      // Note: The java_thread associated with this carrier_thread may not be
      // protected by the ThreadsListHandle above. There could have been an
      // unmount and remount after the ThreadsListHandle above was created
      // and before the JvmtiVTMSTransitionDisabler was created. However, as
      // we have disabled transitions, if we are mounted on it, then it cannot
      // terminate and so is safe to handshake with.

src/hotspot/share/services/threadService.cpp line 1487:

> 1485:     } else {
> 1486:       // We may have previously found a carrier but since unmounted, so
> 1487:       // clear that previous reference.

nit typo: s/but since unmounted/but it since unmounted.

-------------

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26544#pullrequestreview-3072313007
PR Review Comment: https://git.openjdk.org/jdk/pull/26544#discussion_r2243188114
PR Review Comment: https://git.openjdk.org/jdk/pull/26544#discussion_r2243178676