RFR: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage

[David Holmes]

On Fri, 2 Feb 2024 00:44:00 GMT, Serguei Spitsyn <sspitsyn at 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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1475436352