From 1f4fae8087c009451ffd0f5e7c12cf7f2d5d7ab0 Mon Sep 17 00:00:00 2001 From: farvidsson Date: Thu, 24 Oct 2013 10:02:02 +0200 Subject: [PATCH] 8024423: JVMTI: GetLoadedClasses doesn't enumerate anonymous classes Summary: Rewrite of the getLoadedClasses() method implementation to include anonymous classes. Reviewed-by: coleenp, sspitsyn --- src/share/vm/classfile/classLoaderData.cpp | 17 +++ src/share/vm/classfile/classLoaderData.hpp | 2 + src/share/vm/oops/instanceKlass.cpp | 29 ++++- src/share/vm/prims/jvmtiGetLoadedClasses.cpp | 109 +++++++++---------- 4 files changed, 97 insertions(+), 60 deletions(-) diff --git a/src/share/vm/classfile/classLoaderData.cpp b/src/share/vm/classfile/classLoaderData.cpp index 84c191e33..e3209ae18 100644 --- a/src/share/vm/classfile/classLoaderData.cpp +++ b/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()) { diff --git a/src/share/vm/classfile/classLoaderData.hpp b/src/share/vm/classfile/classLoaderData.hpp index 6d5747483..cee114c75 100644 --- a/src/share/vm/classfile/classLoaderData.hpp +++ b/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 { 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. diff --git a/src/share/vm/oops/instanceKlass.cpp b/src/share/vm/oops/instanceKlass.cpp index a8fa7b3ee..510102d4e 100644 --- a/src/share/vm/oops/instanceKlass.cpp +++ b/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; diff --git a/src/share/vm/prims/jvmtiGetLoadedClasses.cpp b/src/share/vm/prims/jvmtiGetLoadedClasses.cpp index 51cfb384a..f58a5a3d5 100644 --- a/src/share/vm/prims/jvmtiGetLoadedClasses.cpp +++ b/src/share/vm/prims/jvmtiGetLoadedClasses.cpp @@ -29,8 +29,43 @@ #include "runtime/thread.hpp" +// The closure for GetLoadedClasses +class LoadedClassesClosure : public KlassClosure { +private: + Stack _classStack; + JvmtiEnv* _env; + +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; -// The closure for GetLoadedClasses and GetClassLoaderClasses + // 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 -- GitLab