This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Re: [RFA] JVMTI/JDWP GetAllLoadedClasses
Tom Tromey wrote:
"Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
Kyle> In order to avoid problems with circular class loading(try to
Kyle> load a class, then try to add it to the class list, whihc
Kyle> requires WeakReference to be loaded, which needs to be stored in
Kyle> the list, whichi requires WeakReference to be loaded...and so
Kyle> on), until it can be assured that WeakReference is loaded (which
Kyle> is checked in ClassList::setup ()), the classes that have been
Kyle> loaded are stored in a temporary linked list of jclass *. When
Kyle> setup is called, these classes are transferred over to the
Kyle> WeakReference list, and the memory used by this temporary list
Kyle> is freed.
I have dropped the entire ClassList class in favor of a HashSet in
VMClassloader. This does present the problem of an additional java
class to deal with, but I managed to get around this, see below.
There's some code like this already in the VM startup sequence, to
handle handoff to the bootstrap class loader after it is created.
Maybe we could reuse this somehow.
I've taken a long look this and I think you are referring to
_Jv_CopyClassesToSystemLoader. It so happens that this methods does the
exact same type of thing to avoid class loading issues. The problem is
that the list it copies over is not complete (most of the time it is
empty), so it doesn't quite do the trick. This method is, however, an
ideal place to know when it is safe to start using java classes, so I
have done the following:
1. Wait for system_class_list == SYSTEM_LOADER_INITIALIZED, which is set
in _Jv_CopyClassesToSystemLoader, before starting to add to the HashSet
directly.
2. When _Jv_CopyClasses... is called, I use this time to copy over from
my temporary list (now in natClassLoader.cc) and free its memory.
Kyle> + return (::java::util::Collection *) (cls_lst);
Just for future reference, there's no need to cast from a subclass to
a superclass. This can be done implicitly.
That's strange, when I take the cast out I get a "cannot convert from
java::util::HashSet* to java::util::Collection* error", so I have left
it in.
I've attached a new patch.
ChangeLog
2007-04-27 Kyle Galloway <kgallowa@redhat.com>
* jvmti.cc (_Jv_JVMTI_GetLoadedClasses): New method.
* classpath/lib/java/lang/VMClassLoader.class: Rebuilt.
* gnu/classpath/jdwp/natVMVirtualMachine (getAllClasses): Implement.
* java/lang/natClassLoader.cc (_Jv_CopyClassesToSystemLoader): Copy and
free the temporary class list for JVMTI.
(_Jv_PushClass): Add classes to the loaded classes list for JVMTI.
* java/lang/VMClassLoader.java (getAllLoadedClasses): New method.
* java/lang/VMClassLoader.h: Regenerated.
More questions, or is this ok?
Thanks,
Kyle
Index: classpath/lib/java/lang/VMClassLoader.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: jvmti.cc
===================================================================
--- jvmti.cc (revision 124192)
+++ jvmti.cc (working copy)
@@ -37,6 +37,8 @@
#include <java/lang/reflect/Modifier.h>
#include <java/util/Collection.h>
#include <java/util/HashMap.h>
+#include <java/util/HashSet.h>
+#include <java/util/Iterator.h>
#include <java/util/concurrent/locks/Lock.h>
#include <java/util/concurrent/locks/ReentrantReadWriteLock.h>
#include <java/net/URL.h>
@@ -1123,6 +1125,35 @@
}
static jvmtiError JNICALL
+_Jv_JVMTI_GetLoadedClasses (jvmtiEnv *env, jint *num_classes, jclass **classes)
+{
+ REQUIRE_PHASE (env, JVMTI_PHASE_LIVE);
+ NULL_CHECK (num_classes);
+ NULL_CHECK (classes);
+
+ using namespace ::java::lang;
+
+ java::util::HashSet *loaded_classes = VMClassLoader::getAllLoadedClasses ();
+
+ *num_classes = loaded_classes->size ();
+
+ jvmtiError jerr = env->Allocate (*num_classes * sizeof (jclass),
+ reinterpret_cast<unsigned char **> (classes));
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ ::java::util::Iterator *it = loaded_classes->iterator ();
+
+ for (int i = 0; it->hasNext () && i < *num_classes; i++)
+ {
+ (*classes)[i] = reinterpret_cast<Class *>
+ (((ref::WeakReference *)it->next ())->get ());
+ }
+
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
_Jv_JVMTI_GetMaxLocals (jvmtiEnv *env, jmethodID method, jint *max_locals)
{
REQUIRE_PHASE (env, JVMTI_PHASE_START | JVMTI_PHASE_LIVE);
@@ -2126,7 +2157,7 @@
UNIMPLEMENTED, // GetBytecodes
_Jv_JVMTI_IsMethodNative, // IsMethodNative
_Jv_JVMTI_IsMethodSynthetic, // IsMethodSynthetic
- UNIMPLEMENTED, // GetLoadedClasses
+ _Jv_JVMTI_GetLoadedClasses, // GetLoadedClasses
_Jv_JVMTI_GetClassLoaderClasses, // GetClassLoaderClasses
UNIMPLEMENTED, // PopFrame
RESERVED, // reserved81
Index: gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- gnu/classpath/jdwp/natVMVirtualMachine.cc (revision 124192)
+++ gnu/classpath/jdwp/natVMVirtualMachine.cc (working copy)
@@ -451,7 +451,24 @@
gnu::classpath::jdwp::VMVirtualMachine::getAllLoadedClasses (void)
{
using namespace ::java::util;
- return (Collection *) new ArrayList ();
+
+ jclass *classes;
+ jint num_classes;
+
+ jvmtiError err = _jdwp_jvmtiEnv->GetLoadedClasses (&num_classes, &classes);
+ if (err != JVMTI_ERROR_NONE)
+ throw_jvmti_error (err);
+
+ ArrayList *cls_list = new ArrayList (num_classes);
+
+ for (int i = 0; i < num_classes; i++)
+ {
+ cls_list->add (classes[i]);
+ }
+
+ _jdwp_jvmtiEnv->Deallocate (reinterpret_cast<unsigned char *> (classes));
+
+ return (Collection *) cls_list;
}
jint
Index: java/lang/natClassLoader.cc
===================================================================
--- java/lang/natClassLoader.cc (revision 124192)
+++ java/lang/natClassLoader.cc (working copy)
@@ -42,6 +42,8 @@
#include <java/io/Serializable.h>
#include <java/lang/Cloneable.h>
#include <java/util/HashMap.h>
+#include <java/lang/ref/WeakReference.h>
+#include <java/util/HashSet.h>
#include <gnu/gcj/runtime/BootClassLoader.h>
#include <gnu/gcj/runtime/SystemClassLoader.h>
@@ -62,6 +64,16 @@
static jclass loaded_classes[HASH_LEN];
+// Element in the temporary loaded classes linked list for JVMTI.
+struct _Jv_TempClassListElement
+{
+ jclass klass;
+ _Jv_TempClassListElement *next;
+};
+
+// The head of the temporary loaded classes linked list for JVMTI.
+static struct _Jv_TempClassListElement *jvmti_class_list_head = NULL;
+
// This is the root of a linked list of classes
static jclass stack_head;
@@ -380,6 +392,19 @@
void
_Jv_CopyClassesToSystemLoader (gnu::gcj::runtime::SystemClassLoader *loader)
{
+ using namespace ::java::lang;
+
+ // Add any classes loaded before this point to the loaded classes list for
+ // JVMTI. As this is done, the memory used by the list is freed.
+ for (struct _Jv_TempClassListElement *ptr = jvmti_class_list_head;
+ ptr;
+ ptr = ptr->next)
+ {
+ VMClassLoader::loaded_classes->add (new ref::WeakReference (ptr->klass));
+ _Jv_Free (jvmti_class_list_head);
+ jvmti_class_list_head = ptr;
+ }
+
for (jclass klass = system_class_list;
klass;
klass = klass->next_or_version)
@@ -655,6 +680,33 @@
_Jv_PushClass (jclass k)
{
JvSynchronize sync (&java::lang::Class::class$);
+
+ using namespace ::java::lang;
+
+ // Build a list of loaded classes for JVMTI.
+ if (system_class_list == SYSTEM_LOADER_INITIALIZED)
+ VMClassLoader::loaded_classes->add (new ref::WeakReference (k));
+ else
+ {
+ if (!jvmti_class_list_head)
+ {
+ jvmti_class_list_head
+ = reinterpret_cast<struct _Jv_TempClassListElement *>
+ (_Jv_MallocUnchecked (sizeof (struct _Jv_TempClassListElement)));
+ jvmti_class_list_head->next = NULL;
+ }
+ else
+ {
+ struct _Jv_TempClassListElement *tmp
+ = reinterpret_cast<struct _Jv_TempClassListElement *>
+ (_Jv_MallocUnchecked (sizeof (struct _Jv_TempClassListElement)));
+ tmp->next = jvmti_class_list_head;
+ jvmti_class_list_head = tmp;
+ }
+
+ jvmti_class_list_head->klass = k;
+ }
+
jclass tmp = stack_head;
stack_head = k;
k->chain = tmp;
Index: java/lang/VMClassLoader.h
===================================================================
--- java/lang/VMClassLoader.h (revision 124192)
+++ java/lang/VMClassLoader.h (working copy)
@@ -57,6 +57,9 @@
static void initialize(::java::lang::String *);
static ::java::lang::Class * nativeFindClass(::java::lang::String *);
static ::java::lang::ClassLoader * getSystemClassLoader();
+public:
+ static ::java::util::HashSet * getAllLoadedClasses();
+public: // actually package-private
static ::java::security::Permission * protectionDomainPermission;
static ::java::security::ProtectionDomain * unknownProtectionDomain;
static ::java::util::HashMap * definedPackages;
@@ -68,6 +71,7 @@
static const jint LIB_CACHE = 1;
static const jint LIB_NEVER = 2;
public:
+ static ::java::util::HashSet * loaded_classes;
static ::java::lang::Class class$;
};
Index: java/lang/VMClassLoader.java
===================================================================
--- java/lang/VMClassLoader.java (revision 124192)
+++ java/lang/VMClassLoader.java (working copy)
@@ -40,6 +40,7 @@
import gnu.java.util.EmptyEnumeration;
import java.lang.reflect.Constructor;
+import java.lang.ref.WeakReference;
import java.io.File;
import java.io.IOException;
import java.net.URL;
@@ -52,6 +53,7 @@
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.Map;
import java.util.StringTokenizer;
import gnu.gcj.runtime.BootClassLoader;
@@ -99,6 +101,9 @@
private static final int LIB_CACHE = 1;
private static final int LIB_NEVER = 2;
+ // List to hold all loaded classes for JVMTI.
+ public static HashSet loaded_classes = new HashSet ();
+
/**
* Helper to define a class using a string of bytes. This assumes that
* the security checks have already been performed, if necessary.
@@ -345,4 +350,24 @@
return default_sys;
}
+
+ /**
+ * First check if any of the classes in the list have been collected, and if
+ * they have, remove them from the list. It then returns the modified list.
+ *
+ * @return The <code>HashSet</code> of loaded classes.
+ */
+ public static HashSet getAllLoadedClasses()
+ {
+ Iterator it = loaded_classes.iterator();
+
+ while (it.hasNext())
+ {
+ WeakReference ref = (WeakReference) it.next();
+ if (ref.get() == null)
+ it.remove();
+ }
+
+ return loaded_classes;
+ }
}