8024423: JVMTI: GetLoadedClasses doesn't enumerate anonymous classes

Rewrite of the getLoadedClasses() method implementation to include anonymous classes.

Reviewed-by: coleenp, sspitsyn

hotspot/src/share/vm/classfile/classLoaderData.cpp

@@ -131,6 +131,17 @@ void ClassLoaderData::classes_do(void f(Klass * const)) {
   }
 }
 
+void ClassLoaderData::loaded_classes_do(KlassClosure* klass_closure) {
+  // Lock to avoid classes being modified/added/removed during iteration
+  MutexLockerEx ml(metaspace_lock(),  Mutex::_no_safepoint_check_flag);
+  for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
+    // Do not filter ArrayKlass oops here...
+    if (k->oop_is_array() || (k->oop_is_instance() && InstanceKlass::cast(k)->is_loaded())) {
+      klass_closure->do_klass(k);
+    }
+  }
+}
+
 void ClassLoaderData::classes_do(void f(InstanceKlass*)) {
   for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
     if (k->oop_is_instance()) {
@@ -600,6 +611,12 @@ void ClassLoaderDataGraph::classes_do(void f(Klass* const)) {
   }
 }
 
+void ClassLoaderDataGraph::loaded_classes_do(KlassClosure* klass_closure) {
+  for (ClassLoaderData* cld = _head; cld != NULL; cld = cld->next()) {
+    cld->loaded_classes_do(klass_closure);
+  }
+}
+
 void ClassLoaderDataGraph::classes_unloading_do(void f(Klass* const)) {
   assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint!");
   for (ClassLoaderData* cld = _unloading; cld != NULL; cld = cld->next()) {

hotspot/src/share/vm/classfile/classLoaderData.hpp

@@ -78,6 +78,7 @@ class ClassLoaderDataGraph : public AllStatic {
   static void keep_alive_oops_do(OopClosure* blk, KlassClosure* klass_closure, bool must_claim);
   static void classes_do(KlassClosure* klass_closure);
   static void classes_do(void f(Klass* const));
+  static void loaded_classes_do(KlassClosure* klass_closure);
   static void classes_unloading_do(void f(Klass* const));
   static bool do_unloading(BoolObjectClosure* is_alive);
 
@@ -186,6 +187,7 @@ class ClassLoaderData : public CHeapObj<mtClass> {
   bool keep_alive() const       { return _keep_alive; }
   bool is_alive(BoolObjectClosure* is_alive_closure) const;
   void classes_do(void f(Klass*));
+  void loaded_classes_do(KlassClosure* klass_closure);
   void classes_do(void f(InstanceKlass*));
 
   // Deallocate free list during class unloading.

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

@@ -2393,15 +2393,38 @@ address InstanceKlass::static_field_addr(int offset) {
 
 
 const char* InstanceKlass::signature_name() const {
+  int hash_len = 0;
+  char hash_buf[40];
+
+  // If this is an anonymous class, append a hash to make the name unique
+  if (is_anonymous()) {
+    assert(EnableInvokeDynamic, "EnableInvokeDynamic was not set.");
+    intptr_t hash = (java_mirror() != NULL) ? java_mirror()->identity_hash() : 0;
+    sprintf(hash_buf, "/" UINTX_FORMAT, (uintx)hash);
+    hash_len = (int)strlen(hash_buf);
+  }
+
+  // Get the internal name as a c string
   const char* src = (const char*) (name()->as_C_string());
   const int src_length = (int)strlen(src);
-  char* dest = NEW_RESOURCE_ARRAY(char, src_length + 3);
-  int src_index = 0;
+
+  char* dest = NEW_RESOURCE_ARRAY(char, src_length + hash_len + 3);
+
+  // Add L as type indicator
   int dest_index = 0;
   dest[dest_index++] = 'L';
-  while (src_index < src_length) {
+
+  // Add the actual class name
+  for (int src_index = 0; src_index < src_length; ) {
     dest[dest_index++] = src[src_index++];
   }
+
+  // If we have a hash, append it
+  for (int hash_index = 0; hash_index < hash_len; ) {
+    dest[dest_index++] = hash_buf[hash_index++];
+  }
+
+  // Add the semicolon and the NULL
   dest[dest_index++] = ';';
   dest[dest_index] = '\0';
   return dest;

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

@@ -29,8 +29,43 @@
 #include "runtime/thread.hpp"
 
 
+// The closure for GetLoadedClasses
+class LoadedClassesClosure : public KlassClosure {
+private:
+  Stack<jclass, mtInternal> _classStack;
+  JvmtiEnv* _env;
 
-// The closure for GetLoadedClasses and GetClassLoaderClasses
+public:
+  LoadedClassesClosure(JvmtiEnv* env) {
+    _env = env;
+  }
+
+  void do_klass(Klass* k) {
+    // Collect all jclasses
+    _classStack.push((jclass) _env->jni_reference(k->java_mirror()));
+  }
+
+  int extract(jclass* result_list) {
+    // The size of the Stack will be 0 after extract, so get it here
+    int count = (int)_classStack.size();
+    int i = count;
+
+    // Pop all jclasses, fill backwards
+    while (!_classStack.is_empty()) {
+      result_list[--i] = _classStack.pop();
+    }
+
+    // Return the number of elements written
+    return count;
+  }
+
+  // Return current size of the Stack
+  int get_count() {
+    return (int)_classStack.size();
+  }
+};
+
+// The closure for GetClassLoaderClasses
 class JvmtiGetLoadedClassesClosure : public StackObj {
   // Since the SystemDictionary::classes_do callback
   // doesn't pass a closureData pointer,
@@ -165,19 +200,6 @@ class JvmtiGetLoadedClassesClosure : public StackObj {
     }
   }
 
-  // Finally, the static methods that are the callbacks
-  static void increment(Klass* k) {
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
-    if (that->get_initiatingLoader() == NULL) {
-      for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-        that->set_count(that->get_count() + 1);
-      }
-    } else if (k != NULL) {
-      // if initiating loader not null, just include the instance with 1 dimension
-      that->set_count(that->get_count() + 1);
-    }
-  }
-
   static void increment_with_loader(Klass* k, ClassLoaderData* loader_data) {
     JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
     oop class_loader = loader_data->class_loader();
@@ -196,24 +218,6 @@ class JvmtiGetLoadedClassesClosure : public StackObj {
     }
   }
 
-  static void add(Klass* k) {
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
-    if (that->available()) {
-      if (that->get_initiatingLoader() == NULL) {
-        for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-          oop mirror = l->java_mirror();
-          that->set_element(that->get_index(), mirror);
-          that->set_index(that->get_index() + 1);
-        }
-      } else if (k != NULL) {
-        // if initiating loader not null, just include the instance with 1 dimension
-        oop mirror = k->java_mirror();
-        that->set_element(that->get_index(), mirror);
-        that->set_index(that->get_index() + 1);
-      }
-    }
-  }
-
   static void add_with_loader(Klass* k, ClassLoaderData* loader_data) {
     JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
     if (that->available()) {
@@ -255,39 +259,30 @@ class JvmtiGetLoadedClassesClosure : public StackObj {
 
 jvmtiError
 JvmtiGetLoadedClasses::getLoadedClasses(JvmtiEnv *env, jint* classCountPtr, jclass** classesPtr) {
-  // Since SystemDictionary::classes_do only takes a function pointer
-  // and doesn't call back with a closure data pointer,
-  // we can only pass static methods.
 
-  JvmtiGetLoadedClassesClosure closure;
+  LoadedClassesClosure closure(env);
   {
     // To get a consistent list of classes we need MultiArray_lock to ensure
-    // array classes aren't created, and SystemDictionary_lock to ensure that
-    // classes aren't added to the system dictionary,
+    // array classes aren't created.
     MutexLocker ma(MultiArray_lock);
-    MutexLocker sd(SystemDictionary_lock);
 
-    // First, count the classes
-    SystemDictionary::classes_do(&JvmtiGetLoadedClassesClosure::increment);
-    Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::increment);
-    // Next, fill in the classes
-    closure.allocate();
-    SystemDictionary::classes_do(&JvmtiGetLoadedClassesClosure::add);
-    Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::add);
-    // Drop the SystemDictionary_lock, so the results could be wrong from here,
-    // but we still have a snapshot.
+    // Iterate through all classes in ClassLoaderDataGraph
+    // and collect them using the LoadedClassesClosure
+    ClassLoaderDataGraph::loaded_classes_do(&closure);
   }
-  // Post results
+
+  // Return results by extracting the collected contents into a list
+  // allocated via JvmtiEnv
   jclass* result_list;
-  jvmtiError err = env->Allocate(closure.get_count() * sizeof(jclass),
-                                 (unsigned char**)&result_list);
-  if (err != JVMTI_ERROR_NONE) {
-    return err;
+  jvmtiError error = env->Allocate(closure.get_count() * sizeof(jclass),
+                               (unsigned char**)&result_list);
+
+  if (error == JVMTI_ERROR_NONE) {
+    int count = closure.extract(result_list);
+    *classCountPtr = count;
+    *classesPtr = result_list;
   }
-  closure.extract(env, result_list);
-  *classCountPtr = closure.get_count();
-  *classesPtr = result_list;
-  return JVMTI_ERROR_NONE;
+  return error;
 }
 
 jvmtiError