8210422: runtime/modules/ModuleStress/ExportModuleStressTest.java - assertion failed: address not aligned: 0x00000008baadbabe
Summary: CLDG_lock caused safepoint in inconsistent state
Reviewed-by: lfoltan, eosterlund, kbarrett
--- a/src/hotspot/share/classfile/classLoaderData.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/classfile/classLoaderData.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -157,6 +157,7 @@
if (!h_class_loader.is_null()) {
_class_loader = _handles.add(h_class_loader());
_class_loader_klass = h_class_loader->klass();
+ initialize_name(h_class_loader);
}
if (!is_unsafe_anonymous) {
@@ -1077,33 +1078,38 @@
// ClassLoaderData into the java/lang/ClassLoader object as a hidden field
ClassLoaderData* ClassLoaderDataGraph::add_to_graph(Handle loader, bool is_unsafe_anonymous) {
+ assert_lock_strong(ClassLoaderDataGraph_lock);
ClassLoaderData* cld;
- {
- NoSafepointVerifier no_safepoints; // we mustn't GC until we've installed the
- // ClassLoaderData in the loader since the CLD
- // contains oops in _handles that must be walked.
- // GC will find the CLD through the loader after this.
- cld = new ClassLoaderData(loader, is_unsafe_anonymous);
-
- if (!is_unsafe_anonymous) {
- // First, Atomically set it
- ClassLoaderData* old = java_lang_ClassLoader::cmpxchg_loader_data(cld, loader(), NULL);
- if (old != NULL) {
- delete cld;
- // Returns the data.
- return old;
- }
+ // First check if another thread beat us to creating the CLD and installing
+ // it into the loader while we were waiting for the lock.
+ if (!is_unsafe_anonymous && loader.not_null()) {
+ cld = java_lang_ClassLoader::loader_data_acquire(loader());
+ if (cld != NULL) {
+ return cld;
}
}
- MutexLocker ml(ClassLoaderDataGraph_lock);
+ // We mustn't GC until we've installed the ClassLoaderData in the Graph since the CLD
+ // contains oops in _handles that must be walked. GC doesn't walk CLD from the
+ // loader oop in all collections, particularly young collections.
+ NoSafepointVerifier no_safepoints;
- // We won the race, and therefore the task of adding the data to the list of
- // class loader data
+ cld = new ClassLoaderData(loader, is_unsafe_anonymous);
+
+ // First install the new CLD to the Graph.
cld->set_next(_head);
_head = cld;
+
+ // Next associate with the class_loader.
+ if (!is_unsafe_anonymous) {
+ // Use OrderAccess, since readers need to get the loader_data only after
+ // it's added to the Graph
+ java_lang_ClassLoader::release_set_loader_data(loader(), cld);
+ }
+
+ // Lastly log, if requested
LogTarget(Trace, class, loader, data) lt;
if (lt.is_enabled()) {
ResourceMark rm;
@@ -1116,12 +1122,8 @@
}
ClassLoaderData* ClassLoaderDataGraph::add(Handle loader, bool is_unsafe_anonymous) {
+ MutexLocker ml(ClassLoaderDataGraph_lock);
ClassLoaderData* loader_data = add_to_graph(loader, is_unsafe_anonymous);
- // Initialize _name and _name_and_id after the loader data is added to the
- // CLDG because adding the Symbol for _name and _name_and_id might safepoint.
- if (loader.not_null()) {
- loader_data->initialize_name(loader);
- }
return loader_data;
}
--- a/src/hotspot/share/classfile/classLoaderData.inline.hpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/classfile/classLoaderData.inline.hpp Tue Sep 11 09:42:27 2018 -0400
@@ -46,7 +46,7 @@
if (loader == NULL) {
return ClassLoaderData::the_null_class_loader_data();
}
- return java_lang_ClassLoader::loader_data(loader);
+ return java_lang_ClassLoader::loader_data_acquire(loader);
}
inline ClassLoaderData* ClassLoaderData::class_loader_data(oop loader) {
@@ -60,7 +60,7 @@
guarantee(loader() != NULL && oopDesc::is_oop(loader()), "Loader must be oop");
// 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= java_lang_ClassLoader::loader_data(loader());
+ ClassLoaderData* loader_data= java_lang_ClassLoader::loader_data_acquire(loader());
if (loader_data) {
return loader_data;
}
--- a/src/hotspot/share/classfile/classLoaderStats.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/classfile/classLoaderStats.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -139,7 +139,7 @@
void ClassLoaderStatsClosure::addEmptyParents(oop cl) {
- while (cl != NULL && java_lang_ClassLoader::loader_data(cl) == NULL) {
+ while (cl != NULL && java_lang_ClassLoader::loader_data_acquire(cl) == NULL) {
// This classloader has not loaded any classes
ClassLoaderStats** cls_ptr = _stats->get(cl);
if (cls_ptr == NULL) {
--- a/src/hotspot/share/classfile/javaClasses.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/classfile/javaClasses.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -4027,9 +4027,9 @@
int java_lang_ClassLoader::nameAndId_offset = -1;
int java_lang_ClassLoader::unnamedModule_offset = -1;
-ClassLoaderData* java_lang_ClassLoader::loader_data(oop loader) {
+ClassLoaderData* java_lang_ClassLoader::loader_data_acquire(oop loader) {
assert(loader != NULL && oopDesc::is_oop(loader), "loader must be oop");
- return HeapAccess<>::load_at(loader, _loader_data_offset);
+ return HeapAccess<MO_ACQUIRE>::load_at(loader, _loader_data_offset);
}
ClassLoaderData* java_lang_ClassLoader::loader_data_raw(oop loader) {
@@ -4037,9 +4037,9 @@
return RawAccess<>::load_at(loader, _loader_data_offset);
}
-ClassLoaderData* java_lang_ClassLoader::cmpxchg_loader_data(ClassLoaderData* new_data, oop loader, ClassLoaderData* expected_data) {
+void java_lang_ClassLoader::release_set_loader_data(oop loader, ClassLoaderData* new_data) {
assert(loader != NULL && oopDesc::is_oop(loader), "loader must be oop");
- return HeapAccess<>::atomic_cmpxchg_at(new_data, loader, _loader_data_offset, expected_data);
+ HeapAccess<MO_RELEASE>::store_at(loader, _loader_data_offset, new_data);
}
#define CLASSLOADER_FIELDS_DO(macro) \
--- a/src/hotspot/share/classfile/javaClasses.hpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/classfile/javaClasses.hpp Tue Sep 11 09:42:27 2018 -0400
@@ -1322,9 +1322,9 @@
static void compute_offsets();
static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;
- static ClassLoaderData* loader_data(oop loader);
+ static ClassLoaderData* loader_data_acquire(oop loader);
static ClassLoaderData* loader_data_raw(oop loader);
- static ClassLoaderData* cmpxchg_loader_data(ClassLoaderData* new_data, oop loader, ClassLoaderData* expected_data);
+ static void release_set_loader_data(oop loader, ClassLoaderData* new_data);
static oop parent(oop loader);
static oop name(oop loader);
--- a/src/hotspot/share/gc/parallel/psCompactionManager.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -183,7 +183,7 @@
void InstanceClassLoaderKlass::oop_pc_follow_contents(oop obj, ParCompactionManager* cm) {
InstanceKlass::oop_pc_follow_contents(obj, cm);
- ClassLoaderData * const loader_data = java_lang_ClassLoader::loader_data(obj);
+ ClassLoaderData * const loader_data = java_lang_ClassLoader::loader_data_acquire(obj);
if (loader_data != NULL) {
cm->follow_class_loader(loader_data);
}
--- a/src/hotspot/share/jfr/periodic/jfrPeriodic.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/jfr/periodic/jfrPeriodic.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -466,9 +466,9 @@
bool do_entry(oop const& key, ClassLoaderStats* const& cls) {
const ClassLoaderData* this_cld = cls->_class_loader != NULL ?
- java_lang_ClassLoader::loader_data(cls->_class_loader) : (ClassLoaderData*)NULL;
+ java_lang_ClassLoader::loader_data_acquire(cls->_class_loader) : NULL;
const ClassLoaderData* parent_cld = cls->_parent != NULL ?
- java_lang_ClassLoader::loader_data(cls->_parent) : (ClassLoaderData*)NULL;
+ java_lang_ClassLoader::loader_data_acquire(cls->_parent) : NULL;
EventClassLoaderStatistics event;
event.set_classLoader(this_cld);
event.set_parentClassLoader(parent_cld);
--- a/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -129,7 +129,7 @@
// requested, so only need to walk this loader's ClassLoaderData
// dictionary, or the NULL ClassLoaderData dictionary for bootstrap loader.
if (loader != NULL) {
- ClassLoaderData* data = java_lang_ClassLoader::loader_data(loader);
+ ClassLoaderData* data = java_lang_ClassLoader::loader_data_acquire(loader);
// ClassLoader may not be used yet for loading.
if (data != NULL && data->dictionary() != NULL) {
data->dictionary()->all_entries_do(&closure);
--- a/src/hotspot/share/prims/whitebox.cpp Tue Sep 11 09:53:41 2018 -0400
+++ b/src/hotspot/share/prims/whitebox.cpp Tue Sep 11 09:42:27 2018 -0400
@@ -1504,7 +1504,7 @@
oop class_loader_oop = JNIHandles::resolve(class_loader);
ClassLoaderData* cld = class_loader_oop != NULL
- ? java_lang_ClassLoader::loader_data(class_loader_oop)
+ ? java_lang_ClassLoader::loader_data_acquire(class_loader_oop)
: ClassLoaderData::the_null_class_loader_data();
void* metadata = MetadataFactory::new_array<u1>(cld, WhiteBox::array_bytes_to_length((size_t)size), thread);
@@ -1515,7 +1515,7 @@
WB_ENTRY(void, WB_FreeMetaspace(JNIEnv* env, jobject wb, jobject class_loader, jlong addr, jlong size))
oop class_loader_oop = JNIHandles::resolve(class_loader);
ClassLoaderData* cld = class_loader_oop != NULL
- ? java_lang_ClassLoader::loader_data(class_loader_oop)
+ ? java_lang_ClassLoader::loader_data_acquire(class_loader_oop)
: ClassLoaderData::the_null_class_loader_data();
MetadataFactory::free_array(cld, (Array<u1>*)(uintptr_t)addr);