RFR: 8332923: ObjectMonitorUsage.java failed with unexpected waiter_count [v3]

[David Holmes]

On Thu, 30 May 2024 01:13:20 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>>   ObjectMonitorUsage.java failed with `unexpected waiter_count` after [JDK-8328083](https://bugs.openjdk.org/browse/JDK-8328083) on linux x86_32.
>>   There are two changes in this PR:
>> 1. In `JvmtiEnvBase::get_object_monitor_usage` function, change from `java_lang_VirtualThread::is_instance(thread_oop)` to `thread_oop->is_a(vmClasses::BaseVirtualThread_klass())`to support the alternative implementation.
>> 2. In `Threads::get_pending_threads(ThreadsList *, int, address)` function of threads.cpp file, change from `java_lang_VirtualThread::is_instance(thread_oop)` to `thread_oop->is_a(vmClasses::BaseVirtualThread_klass())`to support the alternative implementation. This modified function only used by `JvmtiEnvBase::get_object_monitor_usage(JavaThread*, jobject, jvmtiMonitorUsage*)`, so the risk of the modified on threads.cpp file is low.
>> 
>>   
>> 
>> Additional testing:
>> - [x] linux x86_32 run all testcases in serviceability/jvmti, all testcases run successed expect `serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#default` run failed. This test also run failed before this PR, which has been recorded in [JDK-8333140](https://bugs.openjdk.org/browse/JDK-8333140)
>> - [x] linux x86_64 run all testcases in serviceability/jvmti, all testcases run successed.
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   change from java_lang_VirtualThread::is_instance(thread_oop) to hread_oop->is_a(vmClasses::BaseVirtualThread_klass()) in Threads::get_pending_threads()

src/hotspot/share/prims/jvmtiEnvBase.cpp line 1486:

> 1484:   if (owning_thread != nullptr) {
> 1485:     oop thread_oop = get_vthread_or_thread_oop(owning_thread);
> 1486:     bool is_virtual = thread_oop->is_a(vmClasses::BaseVirtualThread_klass());

It strikes me that this should be handled by `java_lang_VirtualThread::is_instance` based on whether there is continuation support or not. External code like this should not, IMO, needed to know about `BaseVirtualThread`. @AlanBateman  what do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19405#discussion_r1620005105