8173361: various crashes in JvmtiExport::post_compiled_method_load

Don't post information that uses metadata from unloaded nmethods

Reviewed-by: yan
Backport-of: b1d915e

src/hotspot/share/code/nmethod.cpp

@@ -1573,14 +1573,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

@@ -1954,7 +1954,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);
       }
     }
@@ -2004,7 +2004,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

@@ -2153,7 +2153,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++;
@@ -2164,6 +2164,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

@@ -920,9 +920,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;
 }
 
@@ -955,14 +952,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: {
@@ -992,6 +987,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;
 
@@ -1041,3 +1051,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

@@ -480,6 +480,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);
 };
 
 /**
@@ -499,7 +503,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; }
@@ -513,6 +517,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

@@ -262,7 +262,7 @@ void mutex_init() {
 
   def(Patching_lock                , PaddedMutex  , special,     true,  Monitor::_safepoint_check_never);      // used for safepointing and code patching.
   def(Service_lock                 , PaddedMonitor, special,     true,  Monitor::_safepoint_check_never);      // used for service thread operations
-  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always); // used for creating jmethodIDs.
+  def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never); // used for creating jmethodIDs.
 
   def(SystemDictionary_lock        , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);
   def(ProtectionDomainSet_lock     , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);

src/hotspot/share/runtime/serviceThread.cpp

@@ -43,6 +43,7 @@
 #include "services/lowMemoryDetector.hpp"
 
 ServiceThread* ServiceThread::_instance = NULL;
+JvmtiDeferredEvent* ServiceThread::_jvmti_event = NULL;
 
 void ServiceThread::initialize() {
   EXCEPTION_MARK;
@@ -140,7 +141,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;
       }
     }
 
@@ -153,7 +156,8 @@ void ServiceThread::service_thread_entry(JavaThread* jt, TRAPS) {
     }
 
     if (has_jvmti_events) {
-      jvmti_event.post();
+      _jvmti_event->post();
+      _jvmti_event = NULL;  // reset
     }
 
     if (sensors_changed) {
@@ -182,6 +186,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

@@ -29,11 +29,13 @@
 
 // A JavaThread for low memory detection support and JVMTI
 // compiled-method-load events.
+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) {};
@@ -43,9 +45,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.