8247729: GetObjectMonitorUsage() might return inconsistent information

Reviewed-by: dholmes, dcubed, sspitsyn

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -2839,15 +2839,11 @@ JvmtiEnv::GetObjectHashCode(jobject object, jint* hash_code_ptr) {
 // info_ptr - pre-checked for NULL
 jvmtiError
 JvmtiEnv::GetObjectMonitorUsage(jobject object, jvmtiMonitorUsage* info_ptr) {
-  JavaThread* calling_thread = JavaThread::current();
+  // This needs to be performed at a safepoint to gather stable data
-  jvmtiError err = get_object_monitor_usage(calling_thread, object, info_ptr);
+  // because monitor owner / waiters might not be suspended.
-  if (err == JVMTI_ERROR_THREAD_NOT_SUSPENDED) {
+  VM_GetObjectMonitorUsage op(this, JavaThread::current(), object, info_ptr);
-    // Some of the critical threads were not suspended. go to a safepoint and try again
+  VMThread::execute(&op);
-    VM_GetObjectMonitorUsage op(this, calling_thread, object, info_ptr);
+  return op.result();
-    VMThread::execute(&op);
-    err = op.result();
-  }
-  return err;
 } /* end GetObjectMonitorUsage */
 
 

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -937,11 +937,12 @@ JvmtiEnvBase::get_frame_location(JavaThread *java_thread, jint depth,
 
 jvmtiError
 JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject object, jvmtiMonitorUsage* info_ptr) {
-  HandleMark hm;
+  assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
-  Handle hobj;
+  Thread* current_thread = VMThread::vm_thread();
+  assert(current_thread == Thread::current(), "must be");
 
-  Thread* current_thread = Thread::current();
+  HandleMark hm(current_thread);
-  bool at_safepoint = SafepointSynchronize::is_at_safepoint();
+  Handle hobj;
 
   // Check arguments
   {
@@ -952,6 +953,7 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
     hobj = Handle(current_thread, mirror);
   }
 
+  ThreadsListHandle tlh(current_thread);
   JavaThread *owning_thread = NULL;
   ObjectMonitor *mon = NULL;
   jvmtiMonitorUsage ret = {
@@ -962,11 +964,7 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
   // first derive the object's owner and entry_count (if any)
   {
     // Revoke any biases before querying the mark word
-    if (at_safepoint) {
+    BiasedLocking::revoke_at_safepoint(hobj);
-      BiasedLocking::revoke_at_safepoint(hobj);
-    } else {
-      BiasedLocking::revoke(hobj, calling_thread);
-    }
 
     address owner = NULL;
     {
@@ -994,39 +992,19 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
     }
 
     if (owner != NULL) {
-      // Use current thread since function can be called from a
-      // JavaThread or the VMThread.
-      ThreadsListHandle tlh;
       // This monitor is owned so we have to find the owning JavaThread.
       owning_thread = Threads::owning_thread_from_monitor_owner(tlh.list(), owner);
-      // Cannot assume (owning_thread != NULL) here because this function
+      assert(owning_thread != NULL, "owning JavaThread must not be NULL");
-      // may not have been called at a safepoint and the owning_thread
+      Handle     th(current_thread, owning_thread->threadObj());
-      // might not be suspended.
+      ret.owner = (jthread)jni_reference(calling_thread, th);
-      if (owning_thread != NULL) {
+    }
-        // The monitor's owner either has to be the current thread, at safepoint
-        // or it has to be suspended. Any of these conditions will prevent both
-        // contending and waiting threads from modifying the state of
-        // the monitor.
-        if (!at_safepoint && !owning_thread->is_thread_fully_suspended(true, &debug_bits)) {
-          // Don't worry! This return of JVMTI_ERROR_THREAD_NOT_SUSPENDED
-          // will not make it back to the JVM/TI agent. The error code will
-          // get intercepted in JvmtiEnv::GetObjectMonitorUsage() which
-          // will retry the call via a VM_GetObjectMonitorUsage VM op.
-          return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
-        }
-        HandleMark hm;
-        Handle     th(current_thread, owning_thread->threadObj());
-        ret.owner = (jthread)jni_reference(calling_thread, th);
-      }
-      // implied else: no owner
-    } // ThreadsListHandle is destroyed here.
 
     if (owning_thread != NULL) {  // monitor is owned
       // The recursions field of a monitor does not reflect recursions
       // as lightweight locks before inflating the monitor are not included.
       // We have to count the number of recursive monitor entries the hard way.
       // We pass a handle to survive any GCs along the way.
-      ResourceMark rm;
+      ResourceMark rm(current_thread);
       ret.entry_count = count_locked_objects(owning_thread, hobj);
     }
     // implied else: entry_count == 0
@@ -1069,13 +1047,9 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
 
     if (ret.waiter_count > 0) {
       // we have contending and/or waiting threads
-      HandleMark hm;
-      // Use current thread since function can be called from a
-      // JavaThread or the VMThread.
-      ThreadsListHandle tlh;
       if (nWant > 0) {
         // we have contending threads
-        ResourceMark rm;
+        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);
@@ -1085,21 +1059,6 @@ JvmtiEnvBase::get_object_monitor_usage(JavaThread* calling_thread, jobject objec
         }
         for (int i = 0; i < nWant; i++) {
           JavaThread *pending_thread = wantList->at(i);
-          // If the monitor has no owner, then a non-suspended contending
-          // thread could potentially change the state of the monitor by
-          // entering it. The JVM/TI spec doesn't allow this.
-          if (owning_thread == NULL && !at_safepoint &&
-              !pending_thread->is_thread_fully_suspended(true, &debug_bits)) {
-            if (ret.owner != NULL) {
-              destroy_jni_reference(calling_thread, ret.owner);
-            }
-            for (int j = 0; j < i; j++) {
-              destroy_jni_reference(calling_thread, ret.waiters[j]);
-            }
-            deallocate((unsigned char*)ret.waiters);
-            deallocate((unsigned char*)ret.notify_waiters);
-            return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
-          }
           Handle th(current_thread, pending_thread->threadObj());
           ret.waiters[i] = (jthread)jni_reference(calling_thread, th);
         }