On Thu, 30 May 2024 01:13:20 GMT, SendaoYan <s...@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
