8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: reso…

…lving NULL _value"

Add local deferred event list to thread to post events outside CodeCache_lock.

Reviewed-by: yan
Backport-of: 8846a80

src/hotspot/share/code/nmethod.cpp

@@ -49,6 +49,7 @@
 #include "oops/methodData.hpp"
 #include "oops/oop.inline.hpp"
 #include "prims/jvmtiImpl.hpp"
+#include "prims/jvmtiThreadState.hpp"
 #include "runtime/atomic.hpp"
 #include "runtime/flags/flagSetting.hpp"
 #include "runtime/frame.inline.hpp"
@@ -57,6 +58,7 @@
 #include "runtime/orderAccess.hpp"
 #include "runtime/os.hpp"
 #include "runtime/safepointVerifiers.hpp"
+#include "runtime/serviceThread.hpp"
 #include "runtime/sharedRuntime.hpp"
 #include "runtime/sweeper.hpp"
 #include "runtime/vmThread.hpp"
@@ -427,15 +429,15 @@ void nmethod::init_defaults() {
   _has_flushed_dependencies   = 0;
   _lock_count                 = 0;
   _stack_traversal_mark       = 0;
-  _unload_reported            = false; // jvmti state
+  _load_reported              = false; // jvmti state
+  _unload_reported            = false;
   _is_far_code                = false; // nmethods are located in CodeCache
 
 #ifdef ASSERT
   _oops_are_stale             = false;
 #endif
 
   _oops_do_mark_link       = NULL;
-  _jmethod_id              = NULL;
   _osr_link                = NULL;
 #if INCLUDE_RTM_OPT
   _rtm_state               = NoRTM;
@@ -1571,11 +1573,11 @@ void nmethod::flush_dependencies(bool delete_immediately) {
 // post_compiled_method_load_event
 // new method for install_code() path
 // Transfer information from compilation to jvmti
-void nmethod::post_compiled_method_load_event() {
+void nmethod::post_compiled_method_load_event(JvmtiThreadState* state) {
 
- // 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;
+  // 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(
@@ -1587,26 +1589,22 @@ void nmethod::post_compiled_method_load_event() {
       m->signature()->utf8_length(),
       insts_begin(), insts_size());
 
-  if (JvmtiExport::should_post_compiled_method_load() ||
-      JvmtiExport::should_post_compiled_method_unload()) {
-    get_and_cache_jmethod_id();
-  }
 
   if (JvmtiExport::should_post_compiled_method_load()) {
-    // Let the Service thread (which is a real Java thread) post the event
-    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
-    JvmtiDeferredEventQueue::enqueue(
-      JvmtiDeferredEvent::compiled_method_load_event(this));
-  }
-}
-
-jmethodID nmethod::get_and_cache_jmethod_id() {
-  if (_jmethod_id == NULL) {
-    // Cache the jmethod_id since it can no longer be looked up once the
-    // method itself has been marked for unloading.
-    _jmethod_id = method()->jmethod_id();
+    // Only post unload events if load events are found.
+    set_load_reported();
+    // Keep sweeper from turning this into zombie until it is posted.
+    mark_as_seen_on_stack();
+
+    // If a JavaThread hasn't been passed in, let the Service thread
+    // (which is a real Java thread) post the event
+    JvmtiDeferredEvent event = JvmtiDeferredEvent::compiled_method_load_event(this);
+    if (state == NULL) {
+      ServiceThread::enqueue_deferred_event(&event);
+    } else {
+      state->enqueue_event(&event);
+    }
   }
-  return _jmethod_id;
 }
 
 void nmethod::post_compiled_method_unload() {
@@ -1622,18 +1620,17 @@ void nmethod::post_compiled_method_unload() {
   // If a JVMTI agent has enabled the CompiledMethodUnload event then
   // post the event. Sometime later this nmethod will be made a zombie
   // by the sweeper but the Method* will not be valid at that point.
-  // If the _jmethod_id is null then no load event was ever requested
-  // so don't bother posting the unload.  The main reason for this is
-  // that the jmethodID is a weak reference to the Method* so if
+  // The jmethodID is a weak reference to the Method* so if
   // it's being unloaded there's no way to look it up since the weak
   // ref will have been cleared.
-  if (_jmethod_id != NULL && JvmtiExport::should_post_compiled_method_unload()) {
+
+  // Don't bother posting the unload if the load event wasn't posted.
+  if (load_reported() && JvmtiExport::should_post_compiled_method_unload()) {
     assert(!unload_reported(), "already unloaded");
     JvmtiDeferredEvent event =
       JvmtiDeferredEvent::compiled_method_unload_event(this,
-          _jmethod_id, insts_begin());
-    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
-    JvmtiDeferredEventQueue::enqueue(event);
+          method()->jmethod_id(), insts_begin());
+    ServiceThread::enqueue_deferred_event(&event);
   }
 
   // The JVMTI CompiledMethodUnload event can be enabled or disabled at

src/hotspot/share/code/nmethod.hpp

@@ -30,6 +30,7 @@
 class DepChange;
 class DirectiveSet;
 class DebugInformationRecorder;
+class JvmtiThreadState;
 
 // nmethods (native methods) are the compiled code versions of Java methods.
 //
@@ -71,7 +72,6 @@ class nmethod : public CompiledMethod {
 
   // Shared fields for all nmethod's
   int       _entry_bci;        // != InvocationEntryBci if this nmethod is an on-stack replacement method
-  jmethodID _jmethod_id;       // Cache of method()->jmethod_id()
 
   // To support simple linked-list chaining of nmethods:
   nmethod*  _osr_link;         // from InstanceKlass::osr_nmethods_head
@@ -116,8 +116,9 @@ class nmethod : public CompiledMethod {
   // protected by CodeCache_lock
   bool _has_flushed_dependencies;            // Used for maintenance of dependencies (CodeCache_lock)
 
-  // used by jvmti to track if an unload event has been posted for this nmethod.
+  // used by jvmti to track if an event has been posted for this nmethod.
   bool _unload_reported;
+  bool _load_reported;
 
   // Protected by Patching_lock
   volatile signed char _state;               // {not_installed, in_use, not_entrant, zombie, unloaded}
@@ -369,10 +370,6 @@ class nmethod : public CompiledMethod {
   bool  make_not_used()    { return make_not_entrant(); }
   bool  make_zombie()      { return make_not_entrant_or_zombie(zombie); }
 
-  // used by jvmti to track if the unload event has been reported
-  bool  unload_reported()                         { return _unload_reported; }
-  void  set_unload_reported()                     { _unload_reported = true; }
-
   int get_state() const {
     return _state;
   }
@@ -489,6 +486,12 @@ class nmethod : public CompiledMethod {
 
   address* orig_pc_addr(const frame* fr);
 
+  // used by jvmti to track if the load and unload events has been reported
+  bool  unload_reported() const                   { return _unload_reported; }
+  void  set_unload_reported()                     { _unload_reported = true; }
+  bool  load_reported() const                     { return _load_reported; }
+  void  set_load_reported()                       { _load_reported = true; }
+
  public:
   // copying of debugging information
   void copy_scopes_pcs(PcDesc* pcs, int count);
@@ -499,8 +502,7 @@ class nmethod : public CompiledMethod {
   void    set_original_pc(const frame* fr, address pc) { *orig_pc_addr(fr) = pc; }
 
   // jvmti support:
-  void post_compiled_method_load_event();
-  jmethodID get_and_cache_jmethod_id();
+  void post_compiled_method_load_event(JvmtiThreadState* state = NULL);
 
   // verify operations
   void verify();

src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp

@@ -390,7 +390,7 @@ bool ReferenceToThreadRootClosure::do_thread_stack_detailed(JavaThread* jt) {
 
   JvmtiThreadState* const jvmti_thread_state = jt->jvmti_thread_state();
   if (jvmti_thread_state != NULL) {
-    jvmti_thread_state->oops_do(&rcl);
+    jvmti_thread_state->oops_do(&rcl, NULL);
   }
 
   return rcl.complete();

src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp

@@ -32,6 +32,7 @@
 #include "oops/oop.inline.hpp"
 #include "prims/jvmtiCodeBlobEvents.hpp"
 #include "prims/jvmtiExport.hpp"
+#include "prims/jvmtiThreadState.inline.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/vmThread.hpp"
 
@@ -219,25 +220,27 @@ jvmtiError JvmtiCodeBlobEvents::generate_dynamic_code_events(JvmtiEnv* env) {
 
 // Generate a COMPILED_METHOD_LOAD event for each nnmethod
 jvmtiError JvmtiCodeBlobEvents::generate_compiled_method_load_events(JvmtiEnv* env) {
-  HandleMark hm;
-
-  // Walk the CodeCache notifying for live nmethods.  The code cache
-  // may be changing while this is happening which is ok since newly
-  // created nmethod will notify normally and nmethods which are freed
-  // can be safely skipped.
-  MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
-  // Iterate over non-profiled and profiled nmethods
-  NMethodIterator iter(NMethodIterator::only_alive_and_not_unloading);
-  while(iter.next()) {
-    nmethod* current = iter.method();
-    // Lock the nmethod so it can't be freed
-    nmethodLocker nml(current);
-
-    // Don't hold the lock over the notify or jmethodID creation
-    MutexUnlocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
-    current->get_and_cache_jmethod_id();
-    JvmtiExport::post_compiled_method_load(env, current);
+  JvmtiThreadState* state = JvmtiThreadState::state_for(JavaThread::current());
+  {
+    // Walk the CodeCache notifying for live nmethods, don't release the CodeCache_lock
+    // because the sweeper may be running concurrently.
+    // Save events to the queue for posting outside the CodeCache_lock.
+    MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
+    // Iterate over non-profiled and profiled nmethods
+    NMethodIterator iter(NMethodIterator::only_alive_and_not_unloading);
+    while(iter.next()) {
+      nmethod* current = iter.method();
+      current->post_compiled_method_load_event(state);
+    }
   }
+
+  // Now post all the events outside the CodeCache_lock.
+  // If there's a safepoint, the queued events will be kept alive.
+  // Adding these events to the service thread to post is something that
+  // should work, but the service thread doesn't keep up in stress scenarios and
+  // the os eventually kills the process with OOM.
+  // We want this thread to wait until the events are all posted.
+  state->post_events(env);
   return JVMTI_ERROR_NONE;
 }
 

src/hotspot/share/prims/jvmtiExport.cpp

@@ -59,6 +59,7 @@
 #include "runtime/objectMonitor.inline.hpp"
 #include "runtime/os.inline.hpp"
 #include "runtime/safepointVerifiers.hpp"
+#include "runtime/serviceThread.hpp"
 #include "runtime/thread.inline.hpp"
 #include "runtime/threadSMR.hpp"
 #include "runtime/vframe.inline.hpp"
@@ -1352,11 +1353,10 @@ void JvmtiExport::post_class_unload(Klass* klass) {
 
   // postings to the service thread so that it can perform them in a safe
   // context and in-order.
-  MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
   ResourceMark rm;
   // JvmtiDeferredEvent copies the string.
   JvmtiDeferredEvent event = JvmtiDeferredEvent::class_unload_event(klass->name()->as_C_string());
-  JvmtiDeferredEventQueue::enqueue(event);
+  ServiceThread::enqueue_deferred_event(&event);
 }
 
 
@@ -2233,10 +2233,9 @@ void JvmtiExport::post_dynamic_code_generated(const char *name, const void *code
     // It may not be safe to post the event from this thread.  Defer all
     // postings to the service thread so that it can perform them in a safe
     // context and in-order.
-    MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
     JvmtiDeferredEvent event = JvmtiDeferredEvent::dynamic_code_generated_event(
         name, code_begin, code_end);
-    JvmtiDeferredEventQueue::enqueue(event);
+    ServiceThread::enqueue_deferred_event(&event);
   }
 }
 

src/hotspot/share/prims/jvmtiExport.hpp

@@ -167,7 +167,6 @@ class JvmtiExport : public AllStatic {
   // DynamicCodeGenerated events for a given environment.
   friend class JvmtiCodeBlobEvents;
 
-  static void post_compiled_method_load(JvmtiEnv* env, nmethod *nm) NOT_JVMTI_RETURN;
   static void post_dynamic_code_generated(JvmtiEnv* env, const char *name, const void *code_begin,
                                           const void *code_end) NOT_JVMTI_RETURN;
 
@@ -336,6 +335,7 @@ class JvmtiExport : public AllStatic {
                                         unsigned char **data_ptr, unsigned char **end_ptr,
                                         JvmtiCachedClassFileData **cache_ptr) NOT_JVMTI_RETURN_(false);
   static void post_native_method_bind(Method* method, address* function_ptr) NOT_JVMTI_RETURN;
+  static void post_compiled_method_load(JvmtiEnv* env, nmethod *nm) NOT_JVMTI_RETURN;
   static void post_compiled_method_load(nmethod *nm) NOT_JVMTI_RETURN;
   static void post_dynamic_code_generated(const char *name, const void *code_begin, const void *code_end) NOT_JVMTI_RETURN;
 

src/hotspot/share/prims/jvmtiImpl.cpp

@@ -1008,6 +1008,13 @@ void JvmtiDeferredEvent::post() {
   }
 }
 
+void JvmtiDeferredEvent::post_compiled_method_load_event(JvmtiEnv* env) {
+  assert(_type == TYPE_COMPILED_METHOD_LOAD, "only user of this method");
+  nmethod* nm = _event_data.compiled_method_load;
+  JvmtiExport::post_compiled_method_load(env, nm);
+}
+
+
 // 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) {
@@ -1020,20 +1027,14 @@ void JvmtiDeferredEvent::oops_do(OopClosure* f, CodeBlobClosure* cf) {
 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;
-
 bool JvmtiDeferredEventQueue::has_events() {
-  assert(Service_lock->owned_by_self(), "Must own Service_lock");
   return _queue_head != NULL;
 }
 
-void JvmtiDeferredEventQueue::enqueue(const JvmtiDeferredEvent& event) {
-  assert(Service_lock->owned_by_self(), "Must own Service_lock");
-
+void JvmtiDeferredEventQueue::enqueue(JvmtiDeferredEvent event) {
   // Events get added to the end of the queue (and are pulled off the front).
   QueueNode* node = new QueueNode(event);
   if (_queue_tail == NULL) {
@@ -1044,14 +1045,11 @@ void JvmtiDeferredEventQueue::enqueue(const JvmtiDeferredEvent& event) {
     _queue_tail = node;
   }
 
-  Service_lock->notify_all();
   assert((_queue_head == NULL) == (_queue_tail == NULL),
          "Inconsistent queue markers");
 }
 
 JvmtiDeferredEvent JvmtiDeferredEventQueue::dequeue() {
-  assert(Service_lock->owned_by_self(), "Must own Service_lock");
-
   assert(_queue_head != NULL, "Nothing to dequeue");
 
   if (_queue_head == NULL) {
@@ -1073,6 +1071,14 @@ JvmtiDeferredEvent JvmtiDeferredEventQueue::dequeue() {
   return event;
 }
 
+void JvmtiDeferredEventQueue::post(JvmtiEnv* env) {
+  // Post and destroy queue nodes
+  while (_queue_head != NULL) {
+     JvmtiDeferredEvent event = dequeue();
+     event.post_compiled_method_load_event(env);
+  }
+}
+
 void JvmtiDeferredEventQueue::oops_do(OopClosure* f, CodeBlobClosure* cf) {
   for(QueueNode* node = _queue_head; node != NULL; node = node->next()) {
      node->event().oops_do(f, cf);