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

[David Holmes]

On Thu, 30 May 2024 06:31:19 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> 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?
>
> Hopefully the ports will catch up someday and the alternative implementation can be removed. 
> 
> We decided not to rename java.lang.VirtualThread when introducing the alternative implementation as it's just too disruptive. The super class that both implementations extend is BaseVirtualThread so testing for an instance of that is correct for the two implementations.
> 
> If it helps the readability then introducing a function to test if a thread is a virtual thread might help. It could use VMContinuations if needed but right now, testing for an instanceof BaseVirtualThread is okay.

Okay. I still think that should be hidden behind the `java_lang_VirtualThread::is_instance` as it is an implementation detail the JVMTI and thread code shouldn't need to know about IMO. Once the alternative implementation is removed I expect these explicit checks for `BaseVirtualThread` will need to be reverted and we could avoid that if we make a change now.

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

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