8013136: NPG: Parallel class loading tests fail after fix for JDK-8011802
authormgerdin
Wed, 24 Apr 2013 19:55:02 +0200
changeset 17110 a8f03d38bde5
parent 17109 90e6c31bbbe4
child 17111 77f553b98e63
8013136: NPG: Parallel class loading tests fail after fix for JDK-8011802 Summary: Move initialization of dependencies to before allocation of CLD Reviewed-by: stefank, coleenp
hotspot/src/share/vm/classfile/classLoaderData.cpp
hotspot/src/share/vm/classfile/classLoaderData.hpp
--- a/hotspot/src/share/vm/classfile/classLoaderData.cpp	Tue Feb 12 14:15:45 2013 -0800
+++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp	Wed Apr 24 19:55:02 2013 +0200
@@ -66,17 +66,19 @@
 
 ClassLoaderData * ClassLoaderData::_the_null_class_loader_data = NULL;
 
-ClassLoaderData::ClassLoaderData(Handle h_class_loader, bool is_anonymous) :
+ClassLoaderData::ClassLoaderData(Handle h_class_loader, bool is_anonymous, Dependencies dependencies) :
   _class_loader(h_class_loader()),
   _is_anonymous(is_anonymous), _keep_alive(is_anonymous), // initially
   _metaspace(NULL), _unloading(false), _klasses(NULL),
   _claimed(0), _jmethod_ids(NULL), _handles(NULL), _deallocate_list(NULL),
-  _next(NULL), _dependencies(),
+  _next(NULL), _dependencies(dependencies),
   _metaspace_lock(new Mutex(Monitor::leaf+1, "Metaspace allocation lock", true)) {
     // empty
 }
 
 void ClassLoaderData::init_dependencies(TRAPS) {
+  assert(!Universe::is_fully_initialized(), "should only be called when initializing");
+  assert(is_the_null_class_loader_data(), "should only call this for the null class loader");
   _dependencies.init(CHECK);
 }
 
@@ -499,29 +501,25 @@
 // Add a new class loader data node to the list.  Assign the newly created
 // ClassLoaderData into the java/lang/ClassLoader object as a hidden field
 ClassLoaderData* ClassLoaderDataGraph::add(Handle loader, bool is_anonymous, TRAPS) {
-  // Not assigned a class loader data yet.
-  // Create one.
-  ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous);
-  cld->init_dependencies(THREAD);
-  if (HAS_PENDING_EXCEPTION) {
-    delete cld;
-    return NULL;
-  }
+  // We need to allocate all the oops for the ClassLoaderData before allocating the
+  // actual ClassLoaderData object.
+  ClassLoaderData::Dependencies dependencies(CHECK_NULL);
 
-  No_Safepoint_Verifier no_safepoints; // nothing is keeping the dependencies array in cld alive
-                                       // make sure we don't encounter a GC until we've inserted
-                                       // cld into the CLDG
+  No_Safepoint_Verifier no_safepoints; // we mustn't GC until we've installed the
+                                       // ClassLoaderData in the graph since the CLD
+                                       // contains unhandled oops
+
+  ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous, dependencies);
+
 
   if (!is_anonymous) {
     ClassLoaderData** cld_addr = java_lang_ClassLoader::loader_data_addr(loader());
-    if (cld_addr != NULL) {
-      // First, Atomically set it
-      ClassLoaderData* old = (ClassLoaderData*) Atomic::cmpxchg_ptr(cld, cld_addr, NULL);
-      if (old != NULL) {
-        delete cld;
-        // Returns the data.
-        return old;
-      }
+    // First, Atomically set it
+    ClassLoaderData* old = (ClassLoaderData*) Atomic::cmpxchg_ptr(cld, cld_addr, NULL);
+    if (old != NULL) {
+      delete cld;
+      // Returns the data.
+      return old;
     }
   }
 
--- a/hotspot/src/share/vm/classfile/classLoaderData.hpp	Tue Feb 12 14:15:45 2013 -0800
+++ b/hotspot/src/share/vm/classfile/classLoaderData.hpp	Wed Apr 24 19:55:02 2013 +0200
@@ -100,6 +100,9 @@
                     Thread* THREAD);
    public:
     Dependencies() : _list_head(NULL) {}
+    Dependencies(TRAPS) : _list_head(NULL) {
+      init(CHECK);
+    }
     void add(Handle dependency, TRAPS);
     void init(TRAPS);
     void oops_do(OopClosure* f);
@@ -150,7 +153,7 @@
   void set_next(ClassLoaderData* next) { _next = next; }
   ClassLoaderData* next() const        { return _next; }
 
-  ClassLoaderData(Handle h_class_loader, bool is_anonymous);
+  ClassLoaderData(Handle h_class_loader, bool is_anonymous, Dependencies dependencies);
   ~ClassLoaderData();
 
   void set_metaspace(Metaspace* m) { _metaspace = m; }
@@ -190,7 +193,9 @@
   static void init_null_class_loader_data() {
     assert(_the_null_class_loader_data == NULL, "cannot initialize twice");
     assert(ClassLoaderDataGraph::_head == NULL, "cannot initialize twice");
-    _the_null_class_loader_data = new ClassLoaderData((oop)NULL, false);
+
+    // We explicitly initialize the Dependencies object at a later phase in the initialization
+    _the_null_class_loader_data = new ClassLoaderData((oop)NULL, false, Dependencies());
     ClassLoaderDataGraph::_head = _the_null_class_loader_data;
     assert(_the_null_class_loader_data->is_the_null_class_loader_data(), "Must be");
     if (DumpSharedSpaces) {