8173361: various crashes in JvmtiExport::post_compiled_method_load

Don't post information that uses metadata from unloaded nmethods

Reviewed-by: eosterlund, dholmes, sspitsyn

src/hotspot/share/code/nmethod.cpp

@@ -1565,14 +1565,18 @@ void nmethod::flush_dependencies(bool delete_immediately) {
 // Transfer information from compilation to jvmti
 void nmethod::post_compiled_method_load_event() {
 
-  Method* moop = method();
+ // This is a bad time for a safepoint.  We don't want
+ // this nmethod to get unloaded while we're queueing the event.
+ NoSafepointVerifier nsv;
+
+  Method* m = method();
   HOTSPOT_COMPILED_METHOD_LOAD(
-      (char *) moop->klass_name()->bytes(),
-      moop->klass_name()->utf8_length(),
-      (char *) moop->name()->bytes(),
-      moop->name()->utf8_length(),
-      (char *) moop->signature()->bytes(),
-      moop->signature()->utf8_length(),
+      (char *) m->klass_name()->bytes(),
+      m->klass_name()->utf8_length(),
+      (char *) m->name()->bytes(),
+      m->name()->utf8_length(),
+      (char *) m->signature()->bytes(),
+      m->signature()->utf8_length(),
       insts_begin(), insts_size());
 
   if (JvmtiExport::should_post_compiled_method_load() ||

src/hotspot/share/oops/instanceKlass.cpp

@@ -1980,7 +1980,7 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) {
         // we're single threaded or at a safepoint - no locking needed
         get_jmethod_id_length_value(jmeths, idnum, &length, &id);
       } else {
-        MutexLocker ml(JmethodIdCreation_lock);
+        MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
         get_jmethod_id_length_value(jmeths, idnum, &length, &id);
       }
     }
@@ -2030,7 +2030,7 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) {
       id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
                                           &to_dealloc_id, &to_dealloc_jmeths);
     } else {
-      MutexLocker ml(JmethodIdCreation_lock);
+      MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
       id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
                                           &to_dealloc_id, &to_dealloc_jmeths);
     }

src/hotspot/share/prims/jvmtiExport.cpp

@@ -2155,7 +2155,7 @@ jvmtiCompiledMethodLoadInlineRecord* create_inline_record(nmethod* nm) {
     int stackframe = 0;
     for(ScopeDesc* sd = nm->scope_desc_at(p->real_pc(nm));sd != NULL;sd = sd->sender()) {
       // sd->method() can be NULL for stubs but not for nmethods. To be completely robust, include an assert that we should never see a null sd->method()
-      assert(sd->method() != NULL, "sd->method() cannot be null.");
+      guarantee(sd->method() != NULL, "sd->method() cannot be null.");
       record->pcinfo[scope].methods[stackframe] = sd->method()->jmethod_id();
       record->pcinfo[scope].bcis[stackframe] = sd->bci();
       stackframe++;
@@ -2166,6 +2166,7 @@ jvmtiCompiledMethodLoadInlineRecord* create_inline_record(nmethod* nm) {
 }
 
 void JvmtiExport::post_compiled_method_load(nmethod *nm) {
+  guarantee(!nm->is_unloading(), "nmethod isn't unloaded or unloading");
   if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
     return;
   }

src/hotspot/share/prims/jvmtiImpl.cpp

@@ -908,9 +908,6 @@ JvmtiDeferredEvent JvmtiDeferredEvent::compiled_method_load_event(
     nmethod* nm) {
   JvmtiDeferredEvent event = JvmtiDeferredEvent(TYPE_COMPILED_METHOD_LOAD);
   event._event_data.compiled_method_load = nm;
-  // Keep the nmethod alive until the ServiceThread can process
-  // this deferred event.
-  nmethodLocker::lock_nmethod(nm);
   return event;
 }
 
@@ -943,14 +940,12 @@ JvmtiDeferredEvent JvmtiDeferredEvent::dynamic_code_generated_event(
 }
 
 void JvmtiDeferredEvent::post() {
-  assert(ServiceThread::is_service_thread(Thread::current()),
+  assert(Thread::current()->is_service_thread(),
          "Service thread must post enqueued events");
   switch(_type) {
     case TYPE_COMPILED_METHOD_LOAD: {
       nmethod* nm = _event_data.compiled_method_load;
       JvmtiExport::post_compiled_method_load(nm);
-      // done with the deferred event so unlock the nmethod
-      nmethodLocker::unlock_nmethod(nm);
       break;
     }
     case TYPE_COMPILED_METHOD_UNLOAD: {
@@ -980,6 +975,21 @@ void JvmtiDeferredEvent::post() {
   }
 }
 
+// Keep the nmethod for compiled_method_load from being unloaded.
+void JvmtiDeferredEvent::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+  if (cf != NULL && _type == TYPE_COMPILED_METHOD_LOAD) {
+    cf->do_code_blob(_event_data.compiled_method_load);
+  }
+}
+
+// The sweeper calls this and marks the nmethods here on the stack so that
+// they cannot be turned into zombies while in the queue.
+void JvmtiDeferredEvent::nmethods_do(CodeBlobClosure* cf) {
+  if (cf != NULL && _type == TYPE_COMPILED_METHOD_LOAD) {
+    cf->do_code_blob(_event_data.compiled_method_load);
+  }  // May add UNLOAD event but it doesn't work yet.
+}
+
 JvmtiDeferredEventQueue::QueueNode* JvmtiDeferredEventQueue::_queue_tail = NULL;
 JvmtiDeferredEventQueue::QueueNode* JvmtiDeferredEventQueue::_queue_head = NULL;
 
@@ -1029,3 +1039,15 @@ JvmtiDeferredEvent JvmtiDeferredEventQueue::dequeue() {
   delete node;
   return event;
 }
+
+void JvmtiDeferredEventQueue::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+  for(QueueNode* node = _queue_head; node != NULL; node = node->next()) {
+     node->event().oops_do(f, cf);
+  }
+}
+
+void JvmtiDeferredEventQueue::nmethods_do(CodeBlobClosure* cf) {
+  for(QueueNode* node = _queue_head; node != NULL; node = node->next()) {
+     node->event().nmethods_do(cf);
+  }
+}

src/hotspot/share/prims/jvmtiImpl.hpp

@@ -475,6 +475,10 @@ class JvmtiDeferredEvent {
 
   // Actually posts the event.
   void post() NOT_JVMTI_RETURN;
+  // Sweeper support to keep nmethods from being zombied while in the queue.
+  void nmethods_do(CodeBlobClosure* cf);
+  // GC support to keep nmethod from being unloaded while in the queue.
+  void oops_do(OopClosure* f, CodeBlobClosure* cf);
 };
 
 /**
@@ -494,7 +498,7 @@ class JvmtiDeferredEventQueue : AllStatic {
     QueueNode(const JvmtiDeferredEvent& event)
       : _event(event), _next(NULL) {}
 
-    const JvmtiDeferredEvent& event() const { return _event; }
+    JvmtiDeferredEvent& event() { return _event; }
     QueueNode* next() const { return _next; }
 
     void set_next(QueueNode* next) { _next = next; }
@@ -508,6 +512,10 @@ class JvmtiDeferredEventQueue : AllStatic {
   static bool has_events() NOT_JVMTI_RETURN_(false);
   static void enqueue(const JvmtiDeferredEvent& event) NOT_JVMTI_RETURN;
   static JvmtiDeferredEvent dequeue() NOT_JVMTI_RETURN_(JvmtiDeferredEvent());
+  // Sweeper support to keep nmethods from being zombied while in the queue.
+  static void nmethods_do(CodeBlobClosure* cf);
+  // GC support to keep nmethod from being unloaded while in the queue.
+  static void oops_do(OopClosure* f, CodeBlobClosure* cf);
 };
 
 // Utility macro that checks for NULL pointers:

src/hotspot/share/runtime/mutexLocker.cpp

@@ -244,7 +244,7 @@ void mutex_init() {
     Notification_lock = Service_lock;
   }
 
-  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  _safepoint_check_always); // used for creating jmethodIDs.
+  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  _safepoint_check_never); // used for creating jmethodIDs.
 
   def(SystemDictionary_lock        , PaddedMonitor, leaf,        true,  _safepoint_check_always);
   def(ProtectionDomainSet_lock     , PaddedMutex  , leaf-1,      true,  _safepoint_check_never);

src/hotspot/share/runtime/serviceThread.cpp

@@ -46,6 +46,7 @@
 #include "services/threadIdTable.hpp"
 
 ServiceThread* ServiceThread::_instance = NULL;
+JvmtiDeferredEvent* ServiceThread::_jvmti_event = NULL;
 
 void ServiceThread::initialize() {
   EXCEPTION_MARK;
@@ -138,7 +139,9 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
       }
 
       if (has_jvmti_events) {
+        // Get the event under the Service_lock
         jvmti_event = JvmtiDeferredEventQueue::dequeue();
+        _jvmti_event = &jvmti_event;
       }
     }
 
@@ -151,7 +154,8 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     }
 
     if (has_jvmti_events) {
-      jvmti_event.post();
+      _jvmti_event->post();
+      _jvmti_event = NULL;  // reset
     }
 
     if (!UseNotificationThread) {
@@ -186,6 +190,26 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
   }
 }
 
-bool ServiceThread::is_service_thread(Thread* thread) {
-  return thread == _instance;
+void ServiceThread::oops_do(OopClosure* f, CodeBlobClosure* cf) {
+  JavaThread::oops_do(f, cf);
+  // The ServiceThread "owns" the JVMTI Deferred events, scan them here
+  // to keep them alive until they are processed.
+  if (cf != NULL) {
+    if (_jvmti_event != NULL) {
+      _jvmti_event->oops_do(f, cf);
+    }
+    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+    JvmtiDeferredEventQueue::oops_do(f, cf);
+  }
+}
+
+void ServiceThread::nmethods_do(CodeBlobClosure* cf) {
+  JavaThread::nmethods_do(cf);
+  if (cf != NULL) {
+    if (_jvmti_event != NULL) {
+      _jvmti_event->nmethods_do(cf);
+    }
+    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
+    JvmtiDeferredEventQueue::nmethods_do(cf);
+  }
 }

src/hotspot/share/runtime/serviceThread.hpp

@@ -30,12 +30,13 @@
 // A hidden from external view JavaThread for JVMTI compiled-method-load
 // events, oop storage cleanup, and the maintainance of string, symbol,
 // protection domain, and resolved method tables.
+class JvmtiDeferredEvent;
 
 class ServiceThread : public JavaThread {
   friend class VMStructs;
  private:
-
   static ServiceThread* _instance;
+  static JvmtiDeferredEvent* _jvmti_event;
 
   static void service_thread_entry(JavaThread* thread, TRAPS);
   ServiceThread(ThreadFunction entry_point) : JavaThread(entry_point) {};
@@ -45,9 +46,11 @@ class ServiceThread : public JavaThread {
 
   // Hide this thread from external view.
   bool is_hidden_from_external_view() const      { return true; }
+  bool is_service_thread() const                 { return true; }
 
-  // Returns true if the passed thread is the service thread.
-  static bool is_service_thread(Thread* thread);
+  // GC support
+  void oops_do(OopClosure* f, CodeBlobClosure* cf);
+  void nmethods_do(CodeBlobClosure* cf);
 };
 
 #endif // SHARE_RUNTIME_SERVICETHREAD_HPP

src/hotspot/share/runtime/thread.hpp

@@ -480,6 +480,7 @@ class Thread: public ThreadShadow {
   virtual bool is_Java_thread()     const            { return false; }
   virtual bool is_Compiler_thread() const            { return false; }
   virtual bool is_Code_cache_sweeper_thread() const  { return false; }
+  virtual bool is_service_thread() const             { return false; }
   virtual bool is_hidden_from_external_view() const  { return false; }
   virtual bool is_jvmti_agent_thread() const         { return false; }
   // True iff the thread can perform GC operations at a safepoint.