On Fri, 2 Feb 2024 02:22:54 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> The implementation of the JVM TI `GetObjectMonitorUsage` does not match the
>> spec.
>> The function returns the following structure:
>>
>>
>> typedef struct {
>> jthread owner;
>> jint entry_count;
>> jint waiter_count;
>> jthread* waiters;
>> jint notify_waiter_count;
>> jthread* notify_waiters;
>> } jvmtiMonitorUsage;
>>
>>
>> The following four fields are defined this way:
>>
>> waiter_count [jint] The number of threads waiting to own this monitor
>> waiters [jthread*] The waiter_count waiting threads
>> notify_waiter_count [jint] The number of threads waiting to be notified by
>> this monitor
>> notify_waiters [jthread*] The notify_waiter_count threads waiting to be
>> notified
>>
>> The `waiters` has to include all threads waiting to enter the monitor or to
>> re-enter it in `Object.wait()`.
>> The implementation also includes the threads waiting to be notified in
>> `Object.wait()` which is wrong.
>> The `notify_waiters` has to include all threads waiting to be notified in
>> `Object.wait()`.
>> The implementation also includes the threads waiting to re-enter the monitor
>> in `Object.wait()` which is wrong.
>> This update makes it right.
>>
>> The implementation of the JDWP command `ObjectReference.MonitorInfo (5)` is
>> based on the JVM TI `GetObjectMonitorInfo()`. This update has a tweak to
>> keep the existing behavior of this command.
>>
>> The follwoing JVMTI vmTestbase tests are fixed to adopt to the
>> `GetObjectMonitorUsage()` correct behavior:
>>
>> jvmti/GetObjectMonitorUsage/objmonusage001
>> jvmti/GetObjectMonitorUsage/objmonusage003
>>
>>
>> The following JVMTI JCK tests have to be fixed to adopt to correct behavior:
>>
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00101/gomu00101a.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102.html
>> vm/jvmti/GetObjectMonitorUsage/gomu001/gomu00102/gomu00102a.html
>>
>>
>>
>> A JCK bug will be filed and the tests have to be added into the JCK problem
>> list located in the closed repository by a separate sub-task before
>> integration of this update.
>>
>> Also, please see and review the related CSR:
>> [8324677](https://bugs.openjdk.org/browse/JDK-8324677): incorrect
>> implementation of JVM TI GetObjectMonitorUsage
>>
>> Testing:
>> - tested with mach5 tiers 1-6
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1553:
>
>> 1551: Handle th(current_thread, get_vthread_or_thread_oop(w));
>> 1552: if (java_lang_Thread::get_thread_status(w->threadObj()) ==
>> 1553: JavaThreadStatus::BLOCKED_ON_MONITOR_ENTER) {
>
> I don't think this is possible. `BLOCKED_ON_MONITOR_ENTER` is only set after
> the target has been removed from the wait-set and added to the cxq queue -
> see `ObjectMonitor::INotify`. As per the comment just above:
>
> // If the thread was found on the ObjectWaiter list, then
> // it has not been notified.
>
> which means it can't be trying to acquire the monitor either.
Thank you for looking at it and the comment, David! Will check what is possible
here.
Do you have any suggestions?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1477954550
