8247972: incorrect implementation of JVM TI GetObjectMonitorUsage

Reviewed-by: dcubed, lmesnik

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -1495,20 +1495,39 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
 
   jint nWant = 0, nWait = 0;
   markWord mark = hobj->mark();
+  ResourceMark rm(current_thread);
+  GrowableArray<JavaThread*>* wantList = nullptr;
+
   if (mark.has_monitor()) {
     mon = mark.monitor();
     assert(mon != nullptr, "must have monitor");
     // this object has a heavyweight monitor
-    nWant = mon->contentions(); // # of threads contending for monitor
-    nWait = mon->waiters();     // # of threads in Object.wait()
-    ret.waiter_count = nWant + nWait;
-    ret.notify_waiter_count = nWait;
+    nWant = mon->contentions(); // # of threads contending for monitor entry, but not re-entry
+    nWait = mon->waiters();     // # of threads waiting for notification,
+                                // or to re-enter monitor, in Object.wait()
+
+    // Get the actual set of threads trying to enter, or re-enter, the monitor.
+    wantList = Threads::get_pending_threads(tlh.list(), nWant + nWait, (address)mon);
+    nWant = wantList->length();
   } else {
     // this object has a lightweight monitor
-    ret.waiter_count = 0;
-    ret.notify_waiter_count = 0;
   }
 
+  if (mon != nullptr) {
+    // Robustness: the actual waiting list can be smaller.
+    // The nWait count we got from the mon->waiters() may include the re-entering
+    // the monitor threads after being notified. Here we are correcting the actual
+    // number of the waiting threads by excluding those re-entering the monitor.
+    nWait = 0;
+    for (ObjectWaiter* waiter = mon->first_waiter();
+         waiter != nullptr && (nWait == 0 || waiter != mon->first_waiter());
+         waiter = mon->next_waiter(waiter)) {
+      nWait++;
+    }
+  }
+  ret.waiter_count = nWant;
+  ret.notify_waiter_count = nWait;
+
   // Allocate memory for heavyweight and lightweight monitor.
   jvmtiError err;
   err = allocate(ret.waiter_count * sizeof(jthread *), (unsigned char**)&ret.waiters);
@@ -1526,56 +1545,32 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
   if (mon != nullptr) {
     // this object has a heavyweight monitor
 
-    // Number of waiters may actually be less than the waiter count.
-    // So null out memory so that unused memory will be null.
+    // null out memory for robustness
     memset(ret.waiters, 0, ret.waiter_count * sizeof(jthread *));
     memset(ret.notify_waiters, 0, ret.notify_waiter_count * sizeof(jthread *));
 
-    if (ret.waiter_count > 0) {
-      // we have contending and/or waiting threads
-      if (nWant > 0) {
-        // we have contending threads
-        ResourceMark rm(current_thread);
-        // get_pending_threads returns only java thread so we do not need to
-        // check for non java threads.
-        GrowableArray<JavaThread*>* wantList = Threads::get_pending_threads(tlh.list(), nWant, (address)mon);
-        if (wantList->length() < nWant) {
-          // robustness: the pending list has gotten smaller
-          nWant = wantList->length();
-        }
-        for (int i = 0; i < nWant; i++) {
-          JavaThread *pending_thread = wantList->at(i);
-          Handle th(current_thread, get_vthread_or_thread_oop(pending_thread));
-          ret.waiters[i] = (jthread)jni_reference(calling_thread, th);
-        }
+    if (ret.waiter_count > 0) { // we have contending threads waiting to enter/re-enter the monitor
+      // identify threads waiting to enter and re-enter the monitor
+      // get_pending_threads returns only java thread so we do not need to
+      // check for non java threads.
+      for (int i = 0; i < nWant; i++) {
+        JavaThread *pending_thread = wantList->at(i);
+        Handle th(current_thread, get_vthread_or_thread_oop(pending_thread));
+        ret.waiters[i] = (jthread)jni_reference(calling_thread, th);
       }
-      if (nWait > 0) {
-        // we have threads in Object.wait()
-        int offset = nWant;  // add after any contending threads
-        ObjectWaiter *waiter = mon->first_waiter();
-        for (int i = 0, j = 0; i < nWait; i++) {
-          if (waiter == nullptr) {
-            // robustness: the waiting list has gotten smaller
-            nWait = j;
-            break;
-          }
-          JavaThread *w = mon->thread_of_waiter(waiter);
-          if (w != nullptr) {
-            // If the thread was found on the ObjectWaiter list, then
-            // it has not been notified. This thread can't change the
-            // state of the monitor so it doesn't need to be suspended.
-            Handle th(current_thread, get_vthread_or_thread_oop(w));
-            ret.waiters[offset + j] = (jthread)jni_reference(calling_thread, th);
-            ret.notify_waiters[j++] = (jthread)jni_reference(calling_thread, th);
-          }
-          waiter = mon->next_waiter(waiter);
-        }
+    }
+    if (ret.notify_waiter_count > 0) { // we have threads waiting to be notified in Object.wait()
+      ObjectWaiter *waiter = mon->first_waiter();
+      for (int i = 0; i < nWait; i++) {
+        JavaThread *w = mon->thread_of_waiter(waiter);
+        assert(w != nullptr, "sanity check");
+        // If the thread was found on the ObjectWaiter list, then
+        // it has not been notified.
+        Handle th(current_thread, get_vthread_or_thread_oop(w));
+        ret.notify_waiters[i] = (jthread)jni_reference(calling_thread, th);
+        waiter = mon->next_waiter(waiter);
       }
-    } // ThreadsListHandle is destroyed here.
-
-    // Adjust count. nWant and nWait count values may be less than original.
-    ret.waiter_count = nWant + nWait;
-    ret.notify_waiter_count = nWait;
+    }
   } else {
     // this object has a lightweight monitor and we have nothing more
     // to do here because the defaults are just fine.

src/hotspot/share/runtime/threads.cpp

@@ -60,6 +60,7 @@
 #include "oops/symbol.hpp"
 #include "prims/jvm_misc.hpp"
 #include "prims/jvmtiAgentList.hpp"
+#include "prims/jvmtiEnvBase.hpp"
 #include "runtime/arguments.hpp"
 #include "runtime/fieldDescriptor.inline.hpp"
 #include "runtime/flags/jvmFlagLimit.hpp"
@@ -1181,10 +1182,12 @@ void Threads::metadata_handles_do(void f(Metadata*)) {
   threads_do(&handles_closure);
 }
 
-// Get count Java threads that are waiting to enter the specified monitor.
+#if INCLUDE_JVMTI
+// Get count of Java threads that are waiting to enter or re-enter the specified monitor.
 GrowableArray<JavaThread*>* Threads::get_pending_threads(ThreadsList * t_list,
                                                          int count,
                                                          address monitor) {
+  assert(Thread::current()->is_VM_thread(), "Must be the VM thread");
   GrowableArray<JavaThread*>* result = new GrowableArray<JavaThread*>(count);
 
   int i = 0;
@@ -1194,15 +1197,22 @@ GrowableArray<JavaThread*>* Threads::get_pending_threads(ThreadsList * t_list,
     // The first stage of async deflation does not affect any field
     // used by this comparison so the ObjectMonitor* is usable here.
     address pending = (address)p->current_pending_monitor();
-    if (pending == monitor) {             // found a match
+    address waiting = (address)p->current_waiting_monitor();
+    oop thread_oop = JvmtiEnvBase::get_vthread_or_thread_oop(p);
+    bool is_virtual = java_lang_VirtualThread::is_instance(thread_oop);
+    jint state = is_virtual ? JvmtiEnvBase::get_vthread_state(thread_oop, p)
+                            : JvmtiEnvBase::get_thread_state(thread_oop, p);
+    if (pending == monitor || (waiting == monitor &&
+        (state & JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER))
+    ) { // found a match
       if (i < count) result->append(p);   // save the first count matches
       i++;
     }
   }
 
   return result;
 }
-
+#endif // INCLUDE_JVMTI
 
 JavaThread *Threads::owning_thread_from_monitor_owner(ThreadsList * t_list,
                                                       address owner) {

src/hotspot/share/runtime/threads.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -131,7 +131,7 @@ class Threads: AllStatic {
   // Print threads busy compiling, and returns the number of printed threads.
   static unsigned print_threads_compiling(outputStream* st, char* buf, int buflen, bool short_form = false);
 
-  // Get Java threads that are waiting to enter a monitor.
+  // Get count of Java threads that are waiting to enter or re-enter the specified monitor.
   static GrowableArray<JavaThread*>* get_pending_threads(ThreadsList * t_list,
                                                          int count, address monitor);
 

src/java.se/share/data/jdwp/jdwp.spec

@@ -1619,8 +1619,8 @@ JDWP "Java(tm) Debug Wire Protocol"
         (Reply
             (threadObject owner "The monitor owner, or null if it is not currently owned.")
             (int entryCount "The number of times the monitor has been entered.")
-            (Repeat waiters "The number of threads that are waiting for the monitor "
-                            "0 if there is no current owner"
+            (Repeat waiters "The total number of threads that are waiting to enter or re-enter "
+                            "the monitor, or waiting to be notified by the monitor."
                 (threadObject thread "A thread waiting for this monitor.")
             )
         )

src/jdk.jdwp.agent/share/native/libjdwp/ObjectReferenceImpl.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -219,14 +219,19 @@ monitorInfo(PacketInputStream *in, PacketOutputStream *out)
             int i;
             (void)outStream_writeObjectRef(env, out, info.owner);
             (void)outStream_writeInt(out, info.entry_count);
-            (void)outStream_writeInt(out, info.waiter_count);
+            (void)outStream_writeInt(out, info.waiter_count + info.notify_waiter_count);
             for (i = 0; i < info.waiter_count; i++) {
                 (void)outStream_writeObjectRef(env, out, info.waiters[i]);
             }
+            for (i = 0; i < info.notify_waiter_count; i++) {
+                (void)outStream_writeObjectRef(env, out, info.notify_waiters[i]);
+            }
         }
 
         if (info.waiters != NULL )
             jvmtiDeallocate(info.waiters);
+        if (info.notify_waiters != NULL )
+            jvmtiDeallocate(info.notify_waiters);
 
     } END_WITH_LOCAL_REFS(env);
 

test/hotspot/jtreg/TEST.quick-groups

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -969,7 +969,6 @@ vmTestbase_nsk_jvmti_quick = \
   vmTestbase/nsk/jvmti/GetMethodName/methname003/TestDescription.java \
   vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage001/TestDescription.java \
   vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage002/TestDescription.java \
-  vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage003/TestDescription.java \
   vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage004/TestDescription.java \
   vmTestbase/nsk/jvmti/GetObjectMonitorUsage/objmonusage005/TestDescription.java \
   vmTestbase/nsk/jvmti/GetObjectSize/objsize001/TestDescription.java \