# HG changeset patch
# User mgerdin
# Date 1366699195 -7200
# Node ID 1cb4795305b9fb92f888f8f456cd52768d31b427
# Parent  ebded0261dfc1d440b0991566ba0078388cc1f08
8011802: NPG: init_dependencies in class loader data graph can cause invalid CLD
Summary: Restructure initialization of ClassLoaderData to not add a new instance if init_dependencies fail
Reviewed-by: stefank, coleenp

diff -r ebded0261dfc -r 1cb4795305b9 src/share/vm/classfile/classLoaderData.cpp
--- a/src/share/vm/classfile/classLoaderData.cpp	Mon Apr 22 22:00:03 2013 -0700
+++ b/src/share/vm/classfile/classLoaderData.cpp	Tue Apr 23 08:39:55 2013 +0200
@@ -53,6 +53,7 @@
 #include "classfile/metadataOnStackMark.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "code/codeCache.hpp"
+#include "memory/gcLocker.hpp"
 #include "memory/metadataFactory.hpp"
 #include "memory/metaspaceShared.hpp"
 #include "memory/oopFactory.hpp"
@@ -423,7 +424,7 @@
 // These anonymous class loaders are to contain classes used for JSR292
 ClassLoaderData* ClassLoaderData::anonymous_class_loader_data(oop loader, TRAPS) {
   // Add a new class loader data to the graph.
-  return ClassLoaderDataGraph::add(NULL, loader, CHECK_NULL);
+  return ClassLoaderDataGraph::add(loader, true, CHECK_NULL);
 }
 
 const char* ClassLoaderData::loader_name() {
@@ -495,30 +496,40 @@
 ClassLoaderData* ClassLoaderDataGraph::_unloading = NULL;
 ClassLoaderData* ClassLoaderDataGraph::_saved_head = NULL;
 
-
 // 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(ClassLoaderData** cld_addr, Handle loader, TRAPS) {
+ClassLoaderData* ClassLoaderDataGraph::add(Handle loader, bool is_anonymous, TRAPS) {
   // Not assigned a class loader data yet.
   // Create one.
-  ClassLoaderData* *list_head = &_head;
-  ClassLoaderData* next = _head;
+  ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous);
+  cld->init_dependencies(THREAD);
+  if (HAS_PENDING_EXCEPTION) {
+    delete cld;
+    return NULL;
+  }
 
-  bool is_anonymous = (cld_addr == NULL);
-  ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous);
+  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
 
-  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;
+  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;
+      }
     }
   }
 
   // We won the race, and therefore the task of adding the data to the list of
   // class loader data
+  ClassLoaderData** list_head = &_head;
+  ClassLoaderData* next = _head;
+
   do {
     cld->set_next(next);
     ClassLoaderData* exchanged = (ClassLoaderData*)Atomic::cmpxchg_ptr(cld, list_head, next);
@@ -531,10 +542,6 @@
                    cld->loader_name());
         tty->print_cr("]");
       }
-      // Create dependencies after the CLD is added to the list.  Otherwise,
-      // the GC GC will not find the CLD and the _class_loader field will
-      // not be updated.
-      cld->init_dependencies(CHECK_NULL);
       return cld;
     }
     next = exchanged;
diff -r ebded0261dfc -r 1cb4795305b9 src/share/vm/classfile/classLoaderData.hpp
--- a/src/share/vm/classfile/classLoaderData.hpp	Mon Apr 22 22:00:03 2013 -0700
+++ b/src/share/vm/classfile/classLoaderData.hpp	Tue Apr 23 08:39:55 2013 +0200
@@ -62,7 +62,7 @@
   // CMS support.
   static ClassLoaderData* _saved_head;
 
-  static ClassLoaderData* add(ClassLoaderData** loader_data_addr, Handle class_loader, TRAPS);
+  static ClassLoaderData* add(Handle class_loader, bool anonymous, TRAPS);
  public:
   static ClassLoaderData* find_or_create(Handle class_loader, TRAPS);
   static void purge();
diff -r ebded0261dfc -r 1cb4795305b9 src/share/vm/classfile/classLoaderData.inline.hpp
--- a/src/share/vm/classfile/classLoaderData.inline.hpp	Mon Apr 22 22:00:03 2013 -0700
+++ b/src/share/vm/classfile/classLoaderData.inline.hpp	Tue Apr 23 08:39:55 2013 +0200
@@ -43,10 +43,9 @@
   assert(loader() != NULL,"Must be a class loader");
   // Gets the class loader data out of the java/lang/ClassLoader object, if non-null
   // it's already in the loader_data, so no need to add
-  ClassLoaderData** loader_data_addr = java_lang_ClassLoader::loader_data_addr(loader());
-  ClassLoaderData* loader_data_id = *loader_data_addr;
-  if (loader_data_id) {
-     return loader_data_id;
+  ClassLoaderData* loader_data= java_lang_ClassLoader::loader_data(loader());
+  if (loader_data) {
+     return loader_data;
   }
-  return ClassLoaderDataGraph::add(loader_data_addr, loader, THREAD);
+  return ClassLoaderDataGraph::add(loader, false, THREAD);
 }