6453355: 4/4 new No_Safepoint_Verifier uses fail during GC

Summary: (for Serguei) Clean up use of No_Safepoint_Verifier in JVM TI
Reviewed-by: dcubed

src/share/vm/oops/instanceKlass.cpp

@@ -950,7 +950,6 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan
     // These allocations will have to be freed if they are unused.
 
     // Allocate a new array of methods.
-    jmethodID* to_dealloc_jmeths = NULL;
     jmethodID* new_jmeths = NULL;
     if (length <= idnum) {
       // A new array will be needed (unless some other thread beats us to it)
@@ -961,7 +960,6 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan
     }
 
     // Allocate a new method ID.
-    jmethodID to_dealloc_id = NULL;
     jmethodID new_id = NULL;
     if (method_h->is_old() && !method_h->is_obsolete()) {
       // The method passed in is old (but not obsolete), we need to use the current version
@@ -975,14 +973,27 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan
       new_id = JNIHandles::make_jmethod_id(method_h);
     }
 
-    {
+    if (Threads::number_of_threads() == 0 || SafepointSynchronize::is_at_safepoint()) {
+      // No need and unsafe to lock the JmethodIdCreation_lock at safepoint.
+      id = get_jmethod_id(ik_h, idnum, new_id, new_jmeths);
+    } else {
       MutexLocker ml(JmethodIdCreation_lock);
+      id = get_jmethod_id(ik_h, idnum, new_id, new_jmeths);
+    }
+  }
+  return id;
+}
 
-      // We must not go to a safepoint while holding this lock.
-      debug_only(No_Safepoint_Verifier nosafepoints;)
 
-      // Retry lookup after we got the lock
-      jmeths = ik_h->methods_jmethod_ids_acquire();
+jmethodID instanceKlass::get_jmethod_id(instanceKlassHandle ik_h, size_t idnum,
+                                        jmethodID new_id, jmethodID* new_jmeths) {
+  // Retry lookup after we got the lock or ensured we are at safepoint
+  jmethodID* jmeths = ik_h->methods_jmethod_ids_acquire();
+  jmethodID  id                = NULL;
+  jmethodID  to_dealloc_id     = NULL;
+  jmethodID* to_dealloc_jmeths = NULL;
+  size_t     length;
+
   if (jmeths == NULL || (length = (size_t)jmeths[0]) <= idnum) {
     if (jmeths != NULL) {
       // We have grown the array: copy the existing entries, and delete the old array
@@ -1002,14 +1013,12 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan
   } else {
     to_dealloc_id = new_id; // the new id wasn't used, mark it for deallocation
   }
-    }
 
   // Free up unneeded or no longer needed resources
   FreeHeap(to_dealloc_jmeths);
   if (to_dealloc_id != NULL) {
     JNIHandles::destroy_jmethod_id(to_dealloc_id);
   }
-  }
   return id;
 }
 

src/share/vm/oops/instanceKlass.hpp

@@ -432,6 +432,8 @@ class instanceKlass: public Klass {
                                                         _enclosing_method_method_index = method_index; }
 
   // jmethodID support
+  static jmethodID get_jmethod_id(instanceKlassHandle ik_h, size_t idnum,
+                                  jmethodID new_id, jmethodID* new_jmeths);
   static jmethodID jmethod_id_for_impl(instanceKlassHandle ik_h, methodHandle method_h);
   jmethodID jmethod_id_or_null(methodOop method);
 

src/share/vm/runtime/thread.cpp

@@ -1317,10 +1317,6 @@ JavaThread::~JavaThread() {
   ThreadSafepointState::destroy(this);
   if (_thread_profiler != NULL) delete _thread_profiler;
   if (_thread_stat != NULL) delete _thread_stat;
-
-  if (jvmti_thread_state() != NULL) {
-    JvmtiExport::cleanup_thread(this);
-  }
 }
 
 
@@ -1571,6 +1567,10 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
     tlab().make_parsable(true);  // retire TLAB
   }
 
+  if (jvmti_thread_state() != NULL) {
+    JvmtiExport::cleanup_thread(this);
+  }
+
   // Remove from list of active threads list, and notify VM thread if we are the last non-daemon thread
   Threads::remove(this);
 }