8236743: JFR: assert(klass != __null) failed: invariant in ObjectSampleCheckpoint::add_to_leakp_set

Reviewed-by: egahlin

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,8 +35,10 @@
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp"
 #include "jfr/recorder/service/jfrOptionSet.hpp"
 #include "jfr/recorder/stacktrace/jfrStackTraceRepository.hpp"
+#include "jfr/support/jfrMethodLookup.hpp"
 #include "jfr/utilities/jfrHashtable.hpp"
 #include "jfr/utilities/jfrTypes.hpp"
+#include "oops/instanceKlass.inline.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "runtime/safepoint.hpp"
 #include "runtime/thread.hpp"
@@ -108,6 +110,7 @@ void ObjectSampleCheckpoint::on_thread_exit(JavaThread* jt) {
 static GrowableArray<traceid>* unloaded_klass_set = NULL;
 
 static void add_to_unloaded_klass_set(traceid klass_id) {
+  assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
   if (unloaded_klass_set == NULL) {
     unloaded_klass_set = c_heap_allocate_array<traceid>();
   }
@@ -115,14 +118,16 @@ static void add_to_unloaded_klass_set(traceid klass_id) {
 }
 
 static void sort_unloaded_klass_set() {
+  assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
   if (unloaded_klass_set != NULL && unloaded_klass_set->length() > 1) {
     unloaded_klass_set->sort(sort_traceid);
   }
 }
 
 void ObjectSampleCheckpoint::on_klass_unload(const Klass* k) {
+  assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
   assert(k != NULL, "invariant");
-  add_to_unloaded_klass_set(TRACE_ID(k));
+  add_to_unloaded_klass_set(JfrTraceId::get(k));
 }
 
 template <typename Processor>
@@ -295,29 +300,31 @@ void ObjectSampleCheckpoint::on_rotation(const ObjectSampler* sampler, JfrStackT
   assert(JfrStream_lock->owned_by_self(), "invariant");
   assert(sampler != NULL, "invariant");
   assert(LeakProfiler::is_running(), "invariant");
+  MutexLocker lock(ClassLoaderDataGraph_lock);
+  // the lock is needed to ensure the unload lists do not grow in the middle of inspection.
   install_stack_traces(sampler, stack_trace_repo);
 }
 
-static traceid get_klass_id(traceid method_id) {
+static bool is_klass_unloaded(traceid klass_id) {
+  assert(ClassLoaderDataGraph_lock->owned_by_self(), "invariant");
+  return unloaded_klass_set != NULL && predicate(unloaded_klass_set, klass_id);
+}
+
+static bool is_processed(traceid method_id) {
   assert(method_id != 0, "invariant");
-  return method_id >> TRACE_ID_SHIFT;
-}
-
-static bool is_klass_unloaded(traceid method_id) {
-  return unloaded_klass_set != NULL && predicate(unloaded_klass_set, get_klass_id(method_id));
-}
-
-static bool is_processed(traceid id) {
-  assert(id != 0, "invariant");
   assert(id_set != NULL, "invariant");
-  return mutable_predicate(id_set, id);
+  return mutable_predicate(id_set, method_id);
 }
 
-void ObjectSampleCheckpoint::add_to_leakp_set(const Method* method, traceid method_id) {
-  if (is_processed(method_id) || is_klass_unloaded(method_id)) {
+void ObjectSampleCheckpoint::add_to_leakp_set(const InstanceKlass* ik, traceid method_id) {
+  assert(ik != NULL, "invariant");
+  if (is_processed(method_id) || is_klass_unloaded(JfrMethodLookup::klass_id(method_id))) {
     return;
   }
-  JfrTraceId::set_leakp(method);
+  const Method* const method = JfrMethodLookup::lookup(ik, method_id);
+  assert(method != NULL, "invariant");
+  assert(method->method_holder() == ik, "invariant");
+  JfrTraceId::set_leakp(ik, method);
 }
 
 void ObjectSampleCheckpoint::write_stacktrace(const JfrStackTrace* trace, JfrCheckpointWriter& writer) {
@@ -330,7 +337,7 @@ void ObjectSampleCheckpoint::write_stacktrace(const JfrStackTrace* trace, JfrChe
   for (u4 i = 0; i < trace->_nr_of_frames; ++i) {
     const JfrStackFrame& frame = trace->_frames[i];
     frame.write(writer);
-    add_to_leakp_set(frame._method, frame._methodid);
+    add_to_leakp_set(frame._klass, frame._methodid);
   }
 }
 
@@ -413,6 +420,7 @@ void ObjectSampleCheckpoint::write(const ObjectSampler* sampler, EdgeStore* edge
 }
 
 static void clear_unloaded_klass_set() {
+  assert(ClassLoaderDataGraph_lock->owned_by_self(), "invariant");
   if (unloaded_klass_set != NULL && unloaded_klass_set->is_nonempty()) {
     unloaded_klass_set->clear();
   }

src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,12 +29,12 @@
 #include "jfr/utilities/jfrTypes.hpp"
 
 class EdgeStore;
+class InstanceKlass;
 class JavaThread;
 class JfrCheckpointWriter;
 class JfrStackTrace;
 class JfrStackTraceRepository;
 class Klass;
-class Method;
 class ObjectSample;
 class ObjectSampleMarker;
 class ObjectSampler;
@@ -45,7 +45,7 @@ class ObjectSampleCheckpoint : AllStatic {
   friend class PathToGcRootsOperation;
   friend class StackTraceBlobInstaller;
  private:
-  static void add_to_leakp_set(const Method* method, traceid method_id);
+  static void add_to_leakp_set(const InstanceKlass* ik, traceid method_id);
   static int save_mark_words(const ObjectSampler* sampler, ObjectSampleMarker& marker, bool emit_all);
   static void write_stacktrace(const JfrStackTrace* trace, JfrCheckpointWriter& writer);
   static void write(const ObjectSampler* sampler, EdgeStore* edge_store, bool emit_all, Thread* thread);

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.hpp

@@ -99,7 +99,7 @@ class JfrTraceId : public AllStatic {
   static traceid use(const ClassLoaderData* cld);
 
   // leak profiler
-  static void set_leakp(const Method* method);
+  static void set_leakp(const Klass* klass, const Method* method);
 
   static void remove(const Klass* klass);
   static void restore(const Klass* klass);

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -117,12 +117,18 @@ inline traceid JfrTraceId::use(const ClassLoaderData* cld) {
   return cld->is_unsafe_anonymous() ? 0 : set_used_and_get(cld);
 }
 
-inline void JfrTraceId::set_leakp(const Method* method) {
-  assert(method != NULL, "invariant");
-  const Klass* const klass = method->method_holder();
+inline void JfrTraceId::set_leakp(const Klass* klass, const Method* method) {
   assert(klass != NULL, "invariant");
   assert(METHOD_AND_CLASS_USED_THIS_EPOCH(klass), "invariant");
-  assert(METHOD_FLAG_USED_THIS_EPOCH(method), "invariant");
+  assert(method != NULL, "invariant");
+  assert(klass == method->method_holder(), "invariant");
+  if (METHOD_FLAG_NOT_USED_THIS_EPOCH(method)) {
+    // the method is already logically tagged, just like the klass,
+    // but because of redefinition, the latest Method*
+    // representation might not have a reified tag.
+    SET_METHOD_FLAG_USED_THIS_EPOCH(method);
+    assert(METHOD_FLAG_USED_THIS_EPOCH(method), "invariant");
+  }
   SET_LEAKP(klass);
   SET_METHOD_LEAKP(method);
 }

src/hotspot/share/jfr/recorder/stacktrace/jfrStackTrace.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,7 +27,9 @@
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp"
 #include "jfr/recorder/repository/jfrChunkWriter.hpp"
 #include "jfr/recorder/stacktrace/jfrStackTrace.hpp"
+#include "jfr/support/jfrMethodLookup.hpp"
 #include "memory/allocation.inline.hpp"
+#include "oops/instanceKlass.inline.hpp"
 #include "runtime/vframe.inline.hpp"
 
 static void copy_frames(JfrStackFrame** lhs_frames, u4 length, const JfrStackFrame* rhs_frames) {
@@ -39,11 +41,11 @@ static void copy_frames(JfrStackFrame** lhs_frames, u4 length, const JfrStackFra
   }
 }
 
-JfrStackFrame::JfrStackFrame(const traceid& id, int bci, int type, const Method* method) :
-  _method(method), _methodid(id), _line(0), _bci(bci), _type(type) {}
+JfrStackFrame::JfrStackFrame(const traceid& id, int bci, int type, const InstanceKlass* ik) :
+  _klass(ik), _methodid(id), _line(0), _bci(bci), _type(type) {}
 
-JfrStackFrame::JfrStackFrame(const traceid& id, int bci, int type, int lineno) :
-  _method(NULL), _methodid(id), _line(lineno), _bci(bci), _type(type) {}
+JfrStackFrame::JfrStackFrame(const traceid& id, int bci, int type, int lineno, const InstanceKlass* ik) :
+  _klass(ik), _methodid(id), _line(lineno), _bci(bci), _type(type) {}
 
 JfrStackTrace::JfrStackTrace(JfrStackFrame* frames, u4 max_frames) :
   _next(NULL),
@@ -200,7 +202,7 @@ bool JfrStackTrace::record_thread(JavaThread& thread, frame& frame) {
     const int lineno = method->line_number_from_bci(bci);
     // Can we determine if it's inlined?
     _hash = (_hash << 2) + (unsigned int)(((size_t)mid >> 2) + (bci << 4) + type);
-    _frames[count] = JfrStackFrame(mid, bci, type, method);
+    _frames[count] = JfrStackFrame(mid, bci, type, lineno, method->method_holder());
     st.samples_next();
     count++;
   }
@@ -211,9 +213,12 @@ bool JfrStackTrace::record_thread(JavaThread& thread, frame& frame) {
 }
 
 void JfrStackFrame::resolve_lineno() const {
-  assert(_method, "no method pointer");
+  assert(_klass, "no klass pointer");
   assert(_line == 0, "already have linenumber");
-  _line = _method->line_number_from_bci(_bci);
+  const Method* const method = JfrMethodLookup::lookup(_klass, _methodid);
+  assert(method != NULL, "invariant");
+  assert(method->method_holder() == _klass, "invariant");
+  _line = method->line_number_from_bci(_bci);
 }
 
 void JfrStackTrace::resolve_linenos() const {
@@ -252,7 +257,7 @@ bool JfrStackTrace::record_safe(JavaThread* thread, int skip) {
     }
     // Can we determine if it's inlined?
     _hash = (_hash << 2) + (unsigned int)(((size_t)mid >> 2) + (bci << 4) + type);
-    _frames[count] = JfrStackFrame(mid, bci, type, method);
+    _frames[count] = JfrStackFrame(mid, bci, type, method->method_holder());
     vfs.next();
     count++;
   }

src/hotspot/share/jfr/recorder/stacktrace/jfrStackTrace.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,23 +29,23 @@
 #include "jfr/utilities/jfrTypes.hpp"
 
 class frame;
+class InstanceKlass;
 class JavaThread;
 class JfrCheckpointWriter;
 class JfrChunkWriter;
-class Method;
 
 class JfrStackFrame {
   friend class ObjectSampleCheckpoint;
  private:
-  const Method* _method;
+  const InstanceKlass* _klass;
   traceid _methodid;
   mutable int _line;
   int _bci;
   u1 _type;
 
  public:
-  JfrStackFrame(const traceid& id, int bci, int type, const Method* method);
-  JfrStackFrame(const traceid& id, int bci, int type, int lineno);
+  JfrStackFrame(const traceid& id, int bci, int type, const InstanceKlass* klass);
+  JfrStackFrame(const traceid& id, int bci, int type, int lineno, const InstanceKlass* klass);
 
   bool equals(const JfrStackFrame& rhs) const;
   void write(JfrChunkWriter& cw) const;

src/hotspot/share/jfr/support/jfrMethodLookup.cpp

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
+#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp"
+#include "jfr/support/jfrMethodLookup.hpp"
+#include "oops/instanceKlass.inline.hpp"
+#include "oops/method.inline.hpp"
+
+// The InstanceKlass is assumed to be the method holder for the method to be looked up.
+static const Method* lookup_method(InstanceKlass* ik, int orig_method_id_num) {
+  assert(ik != NULL, "invariant");
+  assert(orig_method_id_num >= 0, "invariant");
+  assert(orig_method_id_num < ik->methods()->length(), "invariant");
+  const Method* const m = ik->method_with_orig_idnum(orig_method_id_num);
+  assert(m != NULL, "invariant");
+  assert(m->orig_method_idnum() == orig_method_id_num, "invariant");
+  assert(!m->is_obsolete(), "invariant");
+  assert(ik == m->method_holder(), "invariant");
+  return m;
+}
+
+const Method* JfrMethodLookup::lookup(const InstanceKlass* ik, traceid method_id) {
+  assert(ik != NULL, "invariant");
+  return lookup_method(const_cast<InstanceKlass*>(ik), method_id_num(method_id));
+}
+
+int JfrMethodLookup::method_id_num(traceid method_id) {
+  return (int)(method_id & METHOD_ID_NUM_MASK);
+}
+
+traceid JfrMethodLookup::method_id(const Method* method) {
+  assert(method != NULL, "invariant");
+  return METHOD_ID(method->method_holder(), method);
+}
+
+traceid JfrMethodLookup::klass_id(traceid method_id) {
+  return method_id >> TRACE_ID_SHIFT;
+}
+
+traceid JfrMethodLookup::klass_id(const Method* method) {
+  return klass_id(method_id(method));
+}

src/hotspot/share/jfr/support/jfrMethodLookup.hpp

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_JFR_SUPPORT_JFRMETHODLOOKUP_HPP
+#define SHARE_JFR_SUPPORT_JFRMETHODLOOKUP_HPP
+
+#include "jfr/utilities/jfrTypes.hpp"
+#include "memory/allocation.hpp"
+
+class InstanceKlass;
+class Method;
+
+class JfrMethodLookup : AllStatic {
+ public:
+  static const Method* lookup(const InstanceKlass* ik, traceid method_id);
+  static traceid method_id(const Method* method);
+  static int method_id_num(traceid method_id);
+  static traceid klass_id(const Method* method);
+  static traceid klass_id(traceid method_id);
+};
+
+#endif // SHARE_JFR_SUPPORT_JFRMETHODLOOKUP_HPP