8230400: Missing constant pool entry for a method in stacktrace

Reviewed-by: egahlin

src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp

@@ -1497,35 +1497,6 @@ static void rewrite_klass_pointer(InstanceKlass*& ik, InstanceKlass* new_ik, Cla
   ik = new_ik;
 }
 
-// During retransform/redefine, copy the Method specific trace flags
-// from the previous ik ("the original klass") to the new ik ("the scratch_klass").
-// The open code for retransform/redefine does not know about these.
-// In doing this migration here, we ensure the new Methods (defined in scratch klass)
-// will carry over trace tags from the old Methods being replaced,
-// ensuring flag/tag continuity while being transparent to open code.
-static void copy_method_trace_flags(const InstanceKlass* the_original_klass, const InstanceKlass* the_scratch_klass) {
-  assert(the_original_klass != NULL, "invariant");
-  assert(the_scratch_klass != NULL, "invariant");
-  assert(the_original_klass->name() == the_scratch_klass->name(), "invariant");
-  const Array<Method*>* old_methods = the_original_klass->methods();
-  const Array<Method*>* new_methods = the_scratch_klass->methods();
-  const bool equal_array_length = old_methods->length() == new_methods->length();
-  // The Method array has the property of being sorted.
-  // If they are the same length, there is a one-to-one mapping.
-  // If they are unequal, there was a method added (currently only
-  // private static methods allowed to be added), use lookup.
-  for (int i = 0; i < old_methods->length(); ++i) {
-    const Method* const old_method = old_methods->at(i);
-    Method* const new_method = equal_array_length ? new_methods->at(i) :
-      the_scratch_klass->find_method(old_method->name(), old_method->signature());
-    assert(new_method != NULL, "invariant");
-    assert(new_method->name() == old_method->name(), "invariant");
-    assert(new_method->signature() == old_method->signature(), "invariant");
-    new_method->set_trace_flags(old_method->trace_flags());
-    assert(new_method->trace_flags() == old_method->trace_flags(), "invariant");
-  }
-}
-
 static bool is_retransforming(const InstanceKlass* ik, TRAPS) {
   assert(ik != NULL, "invariant");
   assert(JdkJfrEvent::is_a(ik), "invariant");
@@ -1533,16 +1504,7 @@ static bool is_retransforming(const InstanceKlass* ik, TRAPS) {
   assert(name != NULL, "invariant");
   Handle class_loader(THREAD, ik->class_loader());
   Handle protection_domain(THREAD, ik->protection_domain());
-  // nota bene: use lock-free dictionary lookup
+  return SystemDictionary::find(name, class_loader, protection_domain, THREAD) != NULL;
-  const InstanceKlass* prev_ik = (const InstanceKlass*)SystemDictionary::find(name, class_loader, protection_domain, THREAD);
-  if (prev_ik == NULL) {
-    return false;
-  }
-  // an existing ik implies a retransform/redefine
-  assert(prev_ik != NULL, "invariant");
-  assert(JdkJfrEvent::is_a(prev_ik), "invariant");
-  copy_method_trace_flags(prev_ik, ik);
-  return true;
 }
 
 // target for JFR_ON_KLASS_CREATION hook
@@ -1571,12 +1533,8 @@ void JfrEventClassTransformer::on_klass_creation(InstanceKlass*& ik, ClassFilePa
     return;
   }
   assert(JdkJfrEvent::is_subklass(ik), "invariant");
-  if (is_retransforming(ik, THREAD)) {
+  if (ik->is_abstract() || is_retransforming(ik, THREAD)) {
-    // not the initial klass load
+    // abstract and scratch classes are not instrumented
-    return;
-  }
-  if (ik->is_abstract()) {
-    // abstract classes are not instrumented
     return;
   }
   ResourceMark rm(THREAD);

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp

@@ -364,13 +364,13 @@ void JfrCheckpointManager::write_type_set() {
   if (!LeakProfiler::is_running()) {
     JfrCheckpointWriter writer(true, true, Thread::current());
     JfrTypeSet::serialize(&writer, NULL, false);
-    return;
+  } else {
+    Thread* const t = Thread::current();
+    JfrCheckpointWriter leakp_writer(false, true, t);
+    JfrCheckpointWriter writer(false, true, t);
+    JfrTypeSet::serialize(&writer, &leakp_writer, false);
+    ObjectSampleCheckpoint::on_type_set(leakp_writer);
   }
-  Thread* const t = Thread::current();
-  JfrCheckpointWriter leakp_writer(false, true, t);
-  JfrCheckpointWriter writer(false, true, t);
-  JfrTypeSet::serialize(&writer, &leakp_writer, false);
-  ObjectSampleCheckpoint::on_type_set(leakp_writer);
 }
 
 void JfrCheckpointManager::write_type_set_for_unloaded_classes() {

src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSet.cpp

@@ -43,6 +43,7 @@
 #include "oops/objArrayKlass.hpp"
 #include "oops/oop.inline.hpp"
 #include "utilities/accessFlags.hpp"
+#include "utilities/bitMap.inline.hpp"
 
 typedef const Klass* KlassPtr;
 typedef const PackageEntry* PkgPtr;
@@ -117,7 +118,7 @@ static traceid package_id(KlassPtr klass, bool leakp) {
 static traceid module_id(PkgPtr pkg, bool leakp) {
   assert(pkg != NULL, "invariant");
   ModPtr module_entry = pkg->module();
-  if (module_entry == NULL || !module_entry->is_named()) {
+  if (module_entry == NULL) {
     return 0;
   }
   if (leakp) {
@@ -136,9 +137,7 @@ static traceid method_id(KlassPtr klass, MethodPtr method) {
 
 static traceid cld_id(CldPtr cld, bool leakp) {
   assert(cld != NULL, "invariant");
-  if (cld->is_unsafe_anonymous()) {
+  assert(!cld->is_unsafe_anonymous(), "invariant");
-    return 0;
-  }
   if (leakp) {
     SET_LEAKP(cld);
   } else {
@@ -153,6 +152,17 @@ static s4 get_flags(const T* ptr) {
   return ptr->access_flags().get_flags();
 }
 
+static bool is_unsafe_anonymous(const Klass* klass) {
+  assert(klass != NULL, "invariant");
+  return klass->is_instance_klass() && ((const InstanceKlass*)klass)->is_unsafe_anonymous();
+}
+
+static ClassLoaderData* get_cld(const Klass* klass) {
+  assert(klass != NULL, "invariant");
+  return is_unsafe_anonymous(klass) ?
+    InstanceKlass::cast(klass)->unsafe_anonymous_host()->class_loader_data() : klass->class_loader_data();
+}
+
 template <typename T>
 static void set_serialized(const T* ptr) {
   assert(ptr != NULL, "invariant");
@@ -184,7 +194,7 @@ static int write_klass(JfrCheckpointWriter* writer, KlassPtr klass, bool leakp)
     assert(theklass->is_typeArray_klass(), "invariant");
   }
   writer->write(artifact_id(klass));
-  writer->write(cld_id(klass->class_loader_data(), leakp));
+  writer->write(cld_id(get_cld(klass), leakp));
   writer->write(mark_symbol(klass, leakp));
   writer->write(pkg_id);
   writer->write(get_flags(klass));
@@ -538,13 +548,22 @@ static void do_class_loader_data(ClassLoaderData* cld) {
   do_previous_epoch_artifact(_subsystem_callback, cld);
 }
 
-class CldFieldSelector {
+class KlassCldFieldSelector {
  public:
   typedef CldPtr TypePtr;
   static TypePtr select(KlassPtr klass) {
     assert(klass != NULL, "invariant");
-    CldPtr cld = klass->class_loader_data();
+    return get_cld(klass);
-    return cld->is_unsafe_anonymous() ? NULL : cld;
+  }
+};
+
+class ModuleCldFieldSelector {
+public:
+  typedef CldPtr TypePtr;
+  static TypePtr select(KlassPtr klass) {
+    assert(klass != NULL, "invariant");
+    ModPtr mod = ModuleFieldSelector::select(klass);
+    return mod != NULL ? mod->loader_data() : NULL;
   }
 };
 
@@ -570,14 +589,18 @@ typedef JfrPredicatedTypeWriterImplHost<CldPtr, CldPredicate, write__classloader
 typedef JfrTypeWriterHost<CldWriterImpl, TYPE_CLASSLOADER> CldWriter;
 typedef CompositeFunctor<CldPtr, CldWriter, ClearArtifact<CldPtr> > CldWriterWithClear;
 typedef JfrArtifactCallbackHost<CldPtr, CldWriterWithClear> CldCallback;
-typedef KlassToFieldEnvelope<CldFieldSelector, CldWriter> KlassCldWriter;
+typedef KlassToFieldEnvelope<KlassCldFieldSelector, CldWriter> KlassCldWriter;
+typedef KlassToFieldEnvelope<ModuleCldFieldSelector, CldWriter> ModuleCldWriter;
+typedef CompositeFunctor<KlassPtr, KlassCldWriter, ModuleCldWriter> KlassAndModuleCldWriter;
 
 typedef LeakPredicate<CldPtr> LeakCldPredicate;
 typedef JfrPredicatedTypeWriterImplHost<CldPtr, LeakCldPredicate, write__classloader__leakp> LeakCldWriterImpl;
 typedef JfrTypeWriterHost<LeakCldWriterImpl, TYPE_CLASSLOADER> LeakCldWriter;
 
 typedef CompositeFunctor<CldPtr, LeakCldWriter, CldWriter> CompositeCldWriter;
-typedef KlassToFieldEnvelope<CldFieldSelector, CompositeCldWriter> KlassCompositeCldWriter;
+typedef KlassToFieldEnvelope<KlassCldFieldSelector, CompositeCldWriter> KlassCompositeCldWriter;
+typedef KlassToFieldEnvelope<ModuleCldFieldSelector, CompositeCldWriter> ModuleCompositeCldWriter;
+typedef CompositeFunctor<KlassPtr, KlassCompositeCldWriter, ModuleCompositeCldWriter> KlassAndModuleCompositeCldWriter;
 typedef CompositeFunctor<CldPtr, CompositeCldWriter, ClearArtifact<CldPtr> > CompositeCldWriterWithClear;
 typedef JfrArtifactCallbackHost<CldPtr, CompositeCldWriterWithClear> CompositeCldCallback;
 
@@ -585,14 +608,16 @@ static void write_classloaders() {
   assert(_writer != NULL, "invariant");
   CldWriter cldw(_writer, _class_unload);
   KlassCldWriter kcw(&cldw);
+  ModuleCldWriter mcw(&cldw);
+  KlassAndModuleCldWriter kmcw(&kcw, &mcw);
   if (current_epoch()) {
-    _artifacts->iterate_klasses(kcw);
+    _artifacts->iterate_klasses(kmcw);
     _artifacts->tally(cldw);
     return;
   }
   assert(previous_epoch(), "invariant");
   if (_leakp_writer == NULL) {
-    _artifacts->iterate_klasses(kcw);
+    _artifacts->iterate_klasses(kmcw);
     ClearArtifact<CldPtr> clear;
     CldWriterWithClear cldwwc(&cldw, &clear);
     CldCallback callback(&cldwwc);
@@ -602,7 +627,9 @@ static void write_classloaders() {
     LeakCldWriter lcldw(_leakp_writer, _class_unload);
     CompositeCldWriter ccldw(&lcldw, &cldw);
     KlassCompositeCldWriter kccldw(&ccldw);
-    _artifacts->iterate_klasses(kccldw);
+    ModuleCompositeCldWriter mccldw(&ccldw);
+    KlassAndModuleCompositeCldWriter kmccldw(&kccldw, &mccldw);
+    _artifacts->iterate_klasses(kmccldw);
     ClearArtifact<CldPtr> clear;
     CompositeCldWriterWithClear ccldwwc(&ccldw, &clear);
     CompositeCldCallback callback(&ccldwwc);
@@ -652,7 +679,31 @@ int write__method__leakp(JfrCheckpointWriter* writer, const void* m) {
   return write_method(writer, method, true);
 }
 
-template <typename MethodCallback, typename KlassCallback, bool leakp>
+class BitMapFilter {
+  ResourceBitMap _bitmap;
+ public:
+  explicit BitMapFilter(int length = 0) : _bitmap((size_t)length) {}
+  bool operator()(size_t idx) {
+    if (_bitmap.size() == 0) {
+      return true;
+    }
+    if (_bitmap.at(idx)) {
+      return false;
+    }
+    _bitmap.set_bit(idx);
+    return true;
+  }
+};
+
+class AlwaysTrue {
+ public:
+  explicit AlwaysTrue(int length = 0) {}
+  bool operator()(size_t idx) {
+    return true;
+  }
+};
+
+template <typename MethodCallback, typename KlassCallback, class Filter, bool leakp>
 class MethodIteratorHost {
  private:
   MethodCallback _method_cb;
@@ -671,13 +722,19 @@ class MethodIteratorHost {
 
   bool operator()(KlassPtr klass) {
     if (_method_used_predicate(klass)) {
-      const InstanceKlass* const ik = InstanceKlass::cast(klass);
+      const InstanceKlass* ik = InstanceKlass::cast(klass);
       const int len = ik->methods()->length();
-      for (int i = 0; i < len; ++i) {
+      Filter filter(ik->previous_versions() != NULL ? len : 0);
-        MethodPtr method = ik->methods()->at(i);
+      while (ik != NULL) {
-        if (_method_flag_predicate(method)) {
+        for (int i = 0; i < len; ++i) {
-          _method_cb(method);
+          MethodPtr method = ik->methods()->at(i);
+          if (_method_flag_predicate(method) && filter(i)) {
+            _method_cb(method);
+          }
         }
+        // There can be multiple versions of the same method running
+        // due to redefinition. Need to inspect the complete set of methods.
+        ik = ik->previous_versions();
       }
     }
     return _klass_cb(klass);
@@ -697,16 +754,23 @@ class Wrapper {
   }
 };
 
+template <typename T>
+class EmptyStub {
+ public:
+  bool operator()(T const& value) { return true; }
+};
+
 typedef SerializePredicate<MethodPtr> MethodPredicate;
 typedef JfrPredicatedTypeWriterImplHost<MethodPtr, MethodPredicate, write__method> MethodWriterImplTarget;
-typedef Wrapper<KlassPtr, Stub> KlassCallbackStub;
+typedef Wrapper<KlassPtr, EmptyStub> KlassCallbackStub;
 typedef JfrTypeWriterHost<MethodWriterImplTarget, TYPE_METHOD> MethodWriterImpl;
-typedef MethodIteratorHost<MethodWriterImpl, KlassCallbackStub, false> MethodWriter;
+typedef MethodIteratorHost<MethodWriterImpl, KlassCallbackStub, BitMapFilter, false> MethodWriter;
 
 typedef LeakPredicate<MethodPtr> LeakMethodPredicate;
 typedef JfrPredicatedTypeWriterImplHost<MethodPtr, LeakMethodPredicate, write__method__leakp> LeakMethodWriterImplTarget;
 typedef JfrTypeWriterHost<LeakMethodWriterImplTarget, TYPE_METHOD> LeakMethodWriterImpl;
-typedef MethodIteratorHost<LeakMethodWriterImpl, KlassCallbackStub, true> LeakMethodWriter;
+typedef MethodIteratorHost<LeakMethodWriterImpl, KlassCallbackStub, BitMapFilter, true> LeakMethodWriter;
+typedef MethodIteratorHost<LeakMethodWriterImpl, KlassCallbackStub, BitMapFilter, true> LeakMethodWriter;
 typedef CompositeFunctor<KlassPtr, LeakMethodWriter, MethodWriter> CompositeMethodWriter;
 
 static void write_methods() {
@@ -832,7 +896,7 @@ void JfrTypeSet::clear() {
 
 typedef Wrapper<KlassPtr, ClearArtifact> ClearKlassBits;
 typedef Wrapper<MethodPtr, ClearArtifact> ClearMethodFlag;
-typedef MethodIteratorHost<ClearMethodFlag, ClearKlassBits, false> ClearKlassAndMethods;
+typedef MethodIteratorHost<ClearMethodFlag, ClearKlassBits, AlwaysTrue, false> ClearKlassAndMethods;
 
 static size_t teardown() {
   assert(_artifacts != NULL, "invariant");

src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeSetUtils.hpp

@@ -98,12 +98,6 @@ class ClearArtifact<const Method*> {
   }
 };
 
-template <typename T>
-class Stub {
- public:
-  bool operator()(T const& value) { return true; }
-};
-
 template <typename T>
 class SerializePredicate {
   bool _class_unload;

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

@@ -38,7 +38,7 @@ static const int low_offset = 7;
 static const int meta_offset = low_offset - 1;
 #endif
 
-inline void set_bits(jbyte bits, jbyte* const dest) {
+inline void set_bits(jbyte bits, jbyte volatile* const dest) {
   assert(dest != NULL, "invariant");
   if (bits != (*dest & bits)) {
     *dest |= bits;

src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -255,20 +255,22 @@ public final class MetadataRepository {
         staleMetadata = true;
     }
 
-    // Lock around setOutput ensures that other threads dosn't
+    // Lock around setOutput ensures that other threads don't
-    // emit event after setOutput and unregister the event class, before a call
+    // emit events after setOutput and unregister the event class, before a call
     // to storeDescriptorInJVM
     synchronized void setOutput(String filename) {
+        if (staleMetadata) {
+            storeDescriptorInJVM();
+        }
         jvm.setOutput(filename);
 
         unregisterUnloaded();
         if (unregistered) {
-            staleMetadata = typeLibrary.clearUnregistered();
+            if (typeLibrary.clearUnregistered()) {
+                storeDescriptorInJVM();
+            }
             unregistered = false;
         }
-        if (staleMetadata) {
-            storeDescriptorInJVM();
-        }
     }
 
     private void unregisterUnloaded() {