8173361: various crashes in JvmtiExport::post_compiled_method_load

Don't post information that uses metadata from unloaded nmethods

Reviewed-by: andrew, sspitsyn
Backport-of: b1d915ef80ebdf511dc2897b20ada78b3dc44241

hotspot/src/share/vm/code/nmethod.cpp

@@ -1656,24 +1656,28 @@ bool nmethod::can_unload(BoolObjectClosure* is_alive, oop* root, bool unloading_
 // 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.
+  No_Safepoint_Verifier nsv;
+
+  Method* m = method();
 #ifndef USDT2
   HS_DTRACE_PROBE8(hotspot, compiled__method__load,
-      moop->klass_name()->bytes(),
-      moop->klass_name()->utf8_length(),
-      moop->name()->bytes(),
-      moop->name()->utf8_length(),
-      moop->signature()->bytes(),
-      moop->signature()->utf8_length(),
+      m->klass_name()->bytes(),
+      m->klass_name()->utf8_length(),
+      m->name()->bytes(),
+      m->name()->utf8_length(),
+      m->signature()->bytes(),
+      m->signature()->utf8_length(),
       insts_begin(), insts_size());
 #else /* USDT2 */
   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());
 #endif /* USDT2 */
 

hotspot/src/share/vm/oops/instanceKlass.cpp

@@ -1786,7 +1786,7 @@ jmethodID InstanceKlass::get_jmethod_id(instanceKlassHandle ik_h, methodHandle m
         // 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);
+        MutexLockerEx ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
         get_jmethod_id_length_value(jmeths, idnum, &length, &id);
       }
     }
@@ -1836,7 +1836,7 @@ jmethodID InstanceKlass::get_jmethod_id(instanceKlassHandle ik_h, methodHandle m
       id = get_jmethod_id_fetch_or_update(ik_h, idnum, new_id, new_jmeths,
                                           &to_dealloc_id, &to_dealloc_jmeths);
     } else {
-      MutexLocker ml(JmethodIdCreation_lock);
+      MutexLockerEx ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
       id = get_jmethod_id_fetch_or_update(ik_h, idnum, new_id, new_jmeths,
                                           &to_dealloc_id, &to_dealloc_jmeths);
     }

hotspot/src/share/vm/prims/jvmtiExport.cpp

@@ -1754,7 +1754,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++;

hotspot/src/share/vm/prims/jvmtiImpl.cpp

@@ -897,9 +897,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;
 }
 
@@ -932,14 +929,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: {
@@ -969,6 +964,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;
 
@@ -1084,3 +1094,15 @@ void JvmtiDeferredEventQueue::process_pending_events() {
     }
   }
 }
+
+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);
+  }
+}

hotspot/src/share/vm/prims/jvmtiImpl.hpp

@@ -492,6 +492,10 @@ class JvmtiDeferredEvent VALUE_OBJ_CLASS_SPEC {
 
   // 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);
 };
 
 /**
@@ -511,7 +515,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; }
@@ -529,6 +533,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);
 
   // Used to enqueue events without using a lock, for times (such as during
   // safepoint) when we can't or don't want to lock the Service_lock.

hotspot/src/share/vm/runtime/serviceThread.cpp

@@ -34,6 +34,7 @@
 #include "services/diagnosticFramework.hpp"
 
 ServiceThread* ServiceThread::_instance = NULL;
+JvmtiDeferredEvent* ServiceThread::_jvmti_event = NULL;
 
 void ServiceThread::initialize() {
   EXCEPTION_MARK;
@@ -112,12 +113,15 @@ 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;
       }
     }
 
     if (has_jvmti_events) {
-      jvmti_event.post();
+      _jvmti_event->post();
+      _jvmti_event = NULL;  // reset
     }
 
     if (sensors_changed) {
@@ -138,6 +142,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, CLDClosure* cld_f, CodeBlobClosure* cf) {
+  JavaThread::oops_do(f, cld_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);
+    }
+    MutexLockerEx 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);
+    }
+    MutexLockerEx ml(Service_lock, Mutex::_no_safepoint_check_flag);
+    JvmtiDeferredEventQueue::nmethods_do(cf);
+  }
 }

hotspot/src/share/vm/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, CLDClosure* cld_f, CodeBlobClosure* cf);
+  void nmethods_do(CodeBlobClosure* cf);
 };
 
 #endif // SHARE_VM_RUNTIME_SERVICETHREAD_HPP

hotspot/src/share/vm/runtime/thread.hpp

@@ -313,6 +313,7 @@ class Thread: public ThreadShadow {
   virtual bool is_VM_thread()       const            { return false; }
   virtual bool is_Java_thread()     const            { return false; }
   virtual bool is_Compiler_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.