From 67428649f40c336f3988b2073677a1a9b75e6d2d Mon Sep 17 00:00:00 2001 From: mgerdin Date: Tue, 23 Apr 2013 08:39:55 +0200 Subject: [PATCH] 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 --- src/share/vm/classfile/classLoaderData.cpp | 45 +++++++++++-------- src/share/vm/classfile/classLoaderData.hpp | 2 +- .../vm/classfile/classLoaderData.inline.hpp | 9 ++-- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/share/vm/classfile/classLoaderData.cpp b/src/share/vm/classfile/classLoaderData.cpp index 75b9f34f2..083b896f9 100644 --- a/src/share/vm/classfile/classLoaderData.cpp +++ b/src/share/vm/classfile/classLoaderData.cpp @@ -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 @@ void ClassLoaderData::free_deallocate_list() { // 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::_head = NULL; 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; - - bool is_anonymous = (cld_addr == NULL); ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous); - - 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; + cld->init_dependencies(THREAD); + if (HAS_PENDING_EXCEPTION) { + delete cld; + return 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 + + 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 @@ ClassLoaderData* ClassLoaderDataGraph::add(ClassLoaderData** cld_addr, Handle lo 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 --git a/src/share/vm/classfile/classLoaderData.hpp b/src/share/vm/classfile/classLoaderData.hpp index e6315182e..e4e342280 100644 --- a/src/share/vm/classfile/classLoaderData.hpp +++ b/src/share/vm/classfile/classLoaderData.hpp @@ -62,7 +62,7 @@ class ClassLoaderDataGraph : public AllStatic { // 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 --git a/src/share/vm/classfile/classLoaderData.inline.hpp b/src/share/vm/classfile/classLoaderData.inline.hpp index b3a5ccf86..018b6761c 100644 --- a/src/share/vm/classfile/classLoaderData.inline.hpp +++ b/src/share/vm/classfile/classLoaderData.inline.hpp @@ -43,10 +43,9 @@ inline ClassLoaderData *ClassLoaderDataGraph::find_or_create(Handle loader, TRAP 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); } -- GitLab