8209821: Make JVMTI GetClassLoaderClasses not walk CLDG

And also added function with KlassClosure to remove the hacks.

Reviewed-by: lfoltan, sspitsyn

src/hotspot/share/classfile/classLoaderData.cpp

@@ -1254,15 +1254,6 @@ void ClassLoaderDataGraph::dictionary_classes_do(void f(InstanceKlass*, TRAPS),
   }
 }
 
-// Walks all entries in the dictionary including entries initiated by this class loader.
-void ClassLoaderDataGraph::dictionary_all_entries_do(void f(InstanceKlass*, ClassLoaderData*)) {
-  Thread* thread = Thread::current();
-  FOR_ALL_DICTIONARY(cld) {
-    Handle holder(thread, cld->holder_phantom());
-    cld->dictionary()->all_entries_do(f);
-  }
-}
-
 void ClassLoaderDataGraph::verify_dictionary() {
   FOR_ALL_DICTIONARY(cld) {
     cld->dictionary()->verify();

src/hotspot/share/classfile/classLoaderData.hpp

@@ -134,9 +134,6 @@ class ClassLoaderDataGraph : public AllStatic {
   // Added for initialize_itable_for_klass to handle exceptions.
   static void dictionary_classes_do(void f(InstanceKlass*, TRAPS), TRAPS);
 
-  // Iterate all classes and their class loaders, including initiating class loaders.
-  static void dictionary_all_entries_do(void f(InstanceKlass*, ClassLoaderData*));
-
   // VM_CounterDecay iteration support
   static InstanceKlass* try_get_next_class();
 

src/hotspot/share/classfile/dictionary.cpp

@@ -330,13 +330,13 @@ void Dictionary::classes_do(void f(InstanceKlass*, TRAPS), TRAPS) {
 }
 
 // All classes, and their class loaders, including initiating class loaders
-void Dictionary::all_entries_do(void f(InstanceKlass*, ClassLoaderData*)) {
+void Dictionary::all_entries_do(KlassClosure* closure) {
   for (int index = 0; index < table_size(); index++) {
     for (DictionaryEntry* probe = bucket(index);
                           probe != NULL;
                           probe = probe->next()) {
       InstanceKlass* k = probe->instance_klass();
-      f(k, loader_data());
+      closure->do_klass(k);
     }
   }
 }

src/hotspot/share/classfile/dictionary.hpp

@@ -74,7 +74,7 @@ public:
 
   void classes_do(void f(InstanceKlass*));
   void classes_do(void f(InstanceKlass*, TRAPS), TRAPS);
-  void all_entries_do(void f(InstanceKlass*, ClassLoaderData*));
+  void all_entries_do(KlassClosure* closure);
   void classes_do(MetaspaceClosure* it);
 
   void unlink();

src/hotspot/share/memory/universe.cpp

@@ -165,14 +165,15 @@ address Universe::_narrow_ptrs_base;
 uint64_t Universe::_narrow_klass_range = (uint64_t(max_juint)+1);
 
 void Universe::basic_type_classes_do(void f(Klass*)) {
-  f(boolArrayKlassObj());
+  for (int i = T_BOOLEAN; i < T_LONG+1; i++) {
-  f(byteArrayKlassObj());
+    f(_typeArrayKlassObjs[i]);
-  f(charArrayKlassObj());
+  }
-  f(intArrayKlassObj());
+}
-  f(shortArrayKlassObj());
+
-  f(longArrayKlassObj());
+void Universe::basic_type_classes_do(KlassClosure *closure) {
-  f(singleArrayKlassObj());
+  for (int i = T_BOOLEAN; i < T_LONG+1; i++) {
-  f(doubleArrayKlassObj());
+    closure->do_klass(_typeArrayKlassObjs[i]);
+  }
 }
 
 void Universe::oops_do(OopClosure* f, bool do_all) {

src/hotspot/share/memory/universe.hpp

@@ -486,6 +486,7 @@ class Universe: AllStatic {
   // Apply "f" to all klasses for basic types (classes not present in
   // SystemDictionary).
   static void basic_type_classes_do(void f(Klass*));
+  static void basic_type_classes_do(KlassClosure* closure);
   static void metaspace_pointers_do(MetaspaceClosure* it);
 
   // Debugging

src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/dictionary.hpp"
 #include "classfile/classLoaderData.inline.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "gc/shared/collectedHeap.hpp"
@@ -39,16 +40,7 @@ private:
   Stack<jclass, mtInternal> _classStack;
   JvmtiEnv* _env;
   Thread*   _cur_thread;
-
+  bool      _dictionary_walk;
-public:
-  LoadedClassesClosure(Thread* thread, JvmtiEnv* env) : _env(env), _cur_thread(thread) {
-    assert(_cur_thread == Thread::current(), "must be current thread");
-  }
-
-  void do_klass(Klass* k) {
-    // Collect all jclasses
-    _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, k->java_mirror())));
-  }
 
   int extract(jclass* result_list) {
     // The size of the Stack will be 0 after extract, so get it here
@@ -68,198 +60,46 @@ public:
   int get_count() {
     return (int)_classStack.size();
   }
-};
 
-// The closure for GetClassLoaderClasses
+public:
-class JvmtiGetLoadedClassesClosure : public StackObj {
+  LoadedClassesClosure(JvmtiEnv* env, bool dictionary_walk) :
-  // Since the ClassLoaderDataGraph::dictionary_all_entries_do callback
+      _env(env),
-  // doesn't pass a closureData pointer,
+      _cur_thread(Thread::current()),
-  // we use a thread-local slot to hold a pointer to
+      _dictionary_walk(dictionary_walk) {
-  // a stack allocated instance of this structure.
- private:
-  jobject _initiatingLoader;
-  int     _count;
-  Handle* _list;
-  int     _index;
-
- private:
-  // Getting and setting the thread local pointer
-  static JvmtiGetLoadedClassesClosure* get_this() {
-    JvmtiGetLoadedClassesClosure* result = NULL;
-    JavaThread* thread = JavaThread::current();
-    result = thread->get_jvmti_get_loaded_classes_closure();
-    return result;
-  }
-  static void set_this(JvmtiGetLoadedClassesClosure* that) {
-    JavaThread* thread = JavaThread::current();
-    thread->set_jvmti_get_loaded_classes_closure(that);
   }
 
- public:
+  void do_klass(Klass* k) {
-  // Constructor/Destructor
+    // Collect all jclasses
-  JvmtiGetLoadedClassesClosure() {
+    _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, k->java_mirror())));
-    JvmtiGetLoadedClassesClosure* that = get_this();
+    if (_dictionary_walk) {
-    assert(that == NULL, "JvmtiGetLoadedClassesClosure in use");
+      // Collect array classes this way when walking the dictionary (because array classes are
-    _initiatingLoader = NULL;
+      // not in the dictionary).
-    _count = 0;
+      for (Klass* l = k->array_klass_or_null(); l != NULL; l = l->array_klass_or_null()) {
-    _list = NULL;
+        _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, l->java_mirror())));
-    _index = 0;
-    set_this(this);
-  }
-
-  JvmtiGetLoadedClassesClosure(jobject initiatingLoader) {
-    JvmtiGetLoadedClassesClosure* that = get_this();
-    assert(that == NULL, "JvmtiGetLoadedClassesClosure in use");
-    _initiatingLoader = initiatingLoader;
-    _count = 0;
-    _list = NULL;
-    _index = 0;
-    set_this(this);
-  }
-
-  ~JvmtiGetLoadedClassesClosure() {
-    JvmtiGetLoadedClassesClosure* that = get_this();
-    assert(that != NULL, "JvmtiGetLoadedClassesClosure not found");
-    set_this(NULL);
-    _initiatingLoader = NULL;
-    _count = 0;
-    if (_list != NULL) {
-      FreeHeap(_list);
-      _list = NULL;
-    }
-    _index = 0;
-  }
-
-  // Accessors.
-  jobject get_initiatingLoader() {
-    return _initiatingLoader;
-  }
-
-  int get_count() {
-    return _count;
-  }
-
-  void set_count(int value) {
-    _count = value;
-  }
-
-  Handle* get_list() {
-    return _list;
-  }
-
-  void set_list(Handle* value) {
-    _list = value;
-  }
-
-  int get_index() {
-    return _index;
-  }
-
-  void set_index(int value) {
-    _index = value;
-  }
-
-  Handle get_element(int index) {
-    if ((_list != NULL) && (index < _count)) {
-      return _list[index];
-    } else {
-      assert(false, "empty get_element");
-      return Handle();
-    }
-  }
-
-  void set_element(int index, Handle value) {
-    if ((_list != NULL) && (index < _count)) {
-      _list[index] = value;
-    } else {
-      assert(false, "bad set_element");
-    }
-  }
-
-  // Other predicates
-  bool available() {
-    return (_list != NULL);
-  }
-
-#ifdef ASSERT
-  // For debugging.
-  void check(int limit) {
-    for (int i = 0; i < limit; i += 1) {
-      assert(Universe::heap()->is_in(get_element(i)()), "check fails");
-    }
-  }
-#endif
-
-  // Public methods that get called within the scope of the closure
-  void allocate() {
-    _list = NEW_C_HEAP_ARRAY(Handle, _count, mtInternal);
-    assert(_list != NULL, "Out of memory");
-    if (_list == NULL) {
-      _count = 0;
-    }
-  }
-
-  void extract(JvmtiEnv *env, jclass* result) {
-    for (int index = 0; index < _count; index += 1) {
-      result[index] = (jclass) env->jni_reference(get_element(index));
-    }
-  }
-
-  static void increment_with_loader(InstanceKlass* k, ClassLoaderData* loader_data) {
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
-    oop class_loader = loader_data->class_loader();
-    if (class_loader == JNIHandles::resolve(that->get_initiatingLoader())) {
-      for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-        that->set_count(that->get_count() + 1);
       }
     }
   }
 
-  static void add_with_loader(InstanceKlass* k, ClassLoaderData* loader_data) {
+  jvmtiError get_result(JvmtiEnv *env, jint* classCountPtr, jclass** classesPtr) {
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
+    // Return results by extracting the collected contents into a list
-    if (that->available()) {
+    // allocated via JvmtiEnv
-      oop class_loader = loader_data->class_loader();
+    jclass* result_list;
-      if (class_loader == JNIHandles::resolve(that->get_initiatingLoader())) {
+    jvmtiError error = env->Allocate(get_count() * sizeof(jclass),
-        Thread *thread = Thread::current();
+                               (unsigned char**)&result_list);
-        for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-          Handle mirror(thread, l->java_mirror());
-          that->set_element(that->get_index(), mirror);
-          that->set_index(that->get_index() + 1);
-        }
-      }
-    }
-  }
 
-  // increment the count for the given basic type array class (and any
+    if (error == JVMTI_ERROR_NONE) {
-  // multi-dimensional arrays). For example, for [B we check for
+      int count = extract(result_list);
-  // [[B, [[[B, .. and the count is incremented for each one that exists.
+      *classCountPtr = count;
-  static void increment_for_basic_type_arrays(Klass* k) {
+      *classesPtr = result_list;
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
-    assert(that != NULL, "no JvmtiGetLoadedClassesClosure");
-    for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-      that->set_count(that->get_count() + 1);
-    }
-  }
-
-  // add the basic type array class and its multi-dimensional array classes to the list
-  static void add_for_basic_type_arrays(Klass* k) {
-    JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this();
-    assert(that != NULL, "no JvmtiGetLoadedClassesClosure");
-    assert(that->available(), "no list");
-    Thread *thread = Thread::current();
-    for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) {
-      Handle mirror(thread, l->java_mirror());
-      that->set_element(that->get_index(), mirror);
-      that->set_index(that->get_index() + 1);
     }
+    return error;
   }
 };
 
-
 jvmtiError
 JvmtiGetLoadedClasses::getLoadedClasses(JvmtiEnv *env, jint* classCountPtr, jclass** classesPtr) {
 
-  LoadedClassesClosure closure(Thread::current(), env);
+  LoadedClassesClosure closure(env, false);
   {
     // To get a consistent list of classes we need MultiArray_lock to ensure
     // array classes aren't created.
@@ -270,56 +110,35 @@ JvmtiGetLoadedClasses::getLoadedClasses(JvmtiEnv *env, jint* classCountPtr, jcla
     ClassLoaderDataGraph::loaded_classes_do(&closure);
   }
 
-  // Return results by extracting the collected contents into a list
+  return closure.get_result(env, classCountPtr, classesPtr);
-  // allocated via JvmtiEnv
-  jclass* result_list;
-  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;
-  }
-  return error;
 }
 
 jvmtiError
 JvmtiGetLoadedClasses::getClassLoaderClasses(JvmtiEnv *env, jobject initiatingLoader,
                                              jint* classCountPtr, jclass** classesPtr) {
-  // Since ClassLoaderDataGraph::dictionary_all_entries_do only takes a function pointer
+
-  // and doesn't call back with a closure data pointer,
+  LoadedClassesClosure closure(env, true);
-  // we can only pass static methods.
-  JvmtiGetLoadedClassesClosure closure(initiatingLoader);
   {
     // To get a consistent list of classes we need MultiArray_lock to ensure
-    // array classes aren't created, and SystemDictionary_lock to ensure that
+    // array classes aren't created during this walk.
-    // classes aren't added to the class loader data dictionaries.
     MutexLocker ma(MultiArray_lock);
     MutexLocker sd(SystemDictionary_lock);
-    // First, count the classes in the class loader data dictionaries which have this loader recorded
+    oop loader = JNIHandles::resolve(initiatingLoader);
-    // as an initiating loader. For basic type arrays this information is not recorded
+    // All classes loaded from this loader as initiating loader are
-    // so GetClassLoaderClasses will return all of the basic type arrays. This is okay
+    // requested, so only need to walk this loader's ClassLoaderData
-    // because the defining loader for basic type arrays is always the boot class loader
+    // dictionary, or the NULL ClassLoaderData dictionary for bootstrap loader.
-    // and these classes are "visible" to all loaders.
+    if (loader != NULL) {
-    ClassLoaderDataGraph::dictionary_all_entries_do(&JvmtiGetLoadedClassesClosure::increment_with_loader);
+      ClassLoaderData* data = java_lang_ClassLoader::loader_data(loader);
-    Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::increment_for_basic_type_arrays);
+      // ClassLoader may not be used yet for loading.
-    // Next, fill in the classes
+      if (data != NULL && data->dictionary() != NULL) {
-    closure.allocate();
+        data->dictionary()->all_entries_do(&closure);
-    ClassLoaderDataGraph::dictionary_all_entries_do(&JvmtiGetLoadedClassesClosure::add_with_loader);
+      }
-    Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::add_for_basic_type_arrays);
+    } else {
-    // Drop the SystemDictionary_lock, so the results could be wrong from here,
+      ClassLoaderData::the_null_class_loader_data()->dictionary()->all_entries_do(&closure);
-    // but we still have a snapshot.
+    }
+    // Get basic arrays for all loaders.
+    Universe::basic_type_classes_do(&closure);
   }
-  // Post results
+
-  jclass* result_list;
+  return closure.get_result(env, classCountPtr, classesPtr);
-  jvmtiError err = env->Allocate(closure.get_count() * sizeof(jclass),
-                                 (unsigned char**)&result_list);
-  if (err != JVMTI_ERROR_NONE) {
-    return err;
-  }
-  closure.extract(env, result_list);
-  *classCountPtr = closure.get_count();
-  *classesPtr = result_list;
-  return JVMTI_ERROR_NONE;
 }

src/hotspot/share/runtime/mutexLocker.cpp

@@ -235,7 +235,7 @@ void mutex_init() {
   def(InlineCacheBuffer_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);
   def(VMStatistic_lock             , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);
   def(ExpandHeap_lock              , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // Used during compilation by VM thread
-  def(JNIHandleBlockFreeList_lock  , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);      // handles are used by VM thread
+  def(JNIHandleBlockFreeList_lock  , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);      // handles are used by VM thread
   def(SignatureHandlerLibrary_lock , PaddedMutex  , leaf,        false, Monitor::_safepoint_check_always);
   def(SymbolArena_lock             , PaddedMutex  , leaf+2,      true,  Monitor::_safepoint_check_never);
   def(StringTable_lock             , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);

src/hotspot/share/runtime/thread.cpp

@@ -1567,7 +1567,6 @@ void JavaThread::initialize() {
   _is_method_handle_return = 0;
   _jvmti_thread_state= NULL;
   _should_post_on_exceptions_flag = JNI_FALSE;
-  _jvmti_get_loaded_classes_closure = NULL;
   _interp_only_mode    = 0;
   _special_runtime_exit_condition = _no_async_condition;
   _pending_async_exception = NULL;

src/hotspot/share/runtime/thread.hpp

@@ -63,7 +63,6 @@ class ThreadsList;
 class ThreadsSMRSupport;
 
 class JvmtiThreadState;
-class JvmtiGetLoadedClassesClosure;
 class ThreadStatistics;
 class ConcurrentLocksDump;
 class ParkEvent;
@@ -1885,8 +1884,6 @@ class JavaThread: public Thread {
   // the specified JavaThread is exiting.
   JvmtiThreadState *jvmti_thread_state() const                                   { return _jvmti_thread_state; }
   static ByteSize jvmti_thread_state_offset()                                    { return byte_offset_of(JavaThread, _jvmti_thread_state); }
-  void set_jvmti_get_loaded_classes_closure(JvmtiGetLoadedClassesClosure* value) { _jvmti_get_loaded_classes_closure = value; }
-  JvmtiGetLoadedClassesClosure* get_jvmti_get_loaded_classes_closure() const     { return _jvmti_get_loaded_classes_closure; }
 
   // JVMTI PopFrame support
   // Setting and clearing popframe_condition
@@ -1938,7 +1935,6 @@ class JavaThread: public Thread {
 
  private:
   JvmtiThreadState *_jvmti_thread_state;
-  JvmtiGetLoadedClassesClosure* _jvmti_get_loaded_classes_closure;
 
   // Used by the interpreter in fullspeed mode for frame pop, method
   // entry, method exit and single stepping support. This field is