OpenJDK / amber / amber
changeset 56650:c5fe45d1281d
8224151: Test vmTestbase/nsk/sysdict/vm/stress/chain/chain007/chain007.java might hang with release binaries
Summary: Fix deadlock on ClassLoaderDataGraph_lock and g1 clear_claimed_marks.
Reviewed-by: tschatzl, lfoltan, eosterlund
| author | coleenp |
|---|---|
| date | Thu, 06 Jun 2019 15:09:17 -0400 |
| parents | eabe64456156 |
| children | a0b92af12baa 93dac461abf5 afb08a216cdb d273dc2ae04b 93b5578ec871 54bb08cb4a20 098fa5ddb308 |
| files | src/hotspot/share/classfile/classLoaderData.hpp src/hotspot/share/classfile/classLoaderDataGraph.cpp src/hotspot/share/classfile/classLoaderDataGraph.hpp src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp src/hotspot/share/runtime/vmStructs.cpp |
| diffstat | 5 files changed, 18 insertions(+), 8 deletions(-) [+] |
line wrap: on
line diff
--- a/src/hotspot/share/classfile/classLoaderData.hpp Thu Jun 06 20:19:03 2019 +0200 +++ b/src/hotspot/share/classfile/classLoaderData.hpp Thu Jun 06 15:09:17 2019 -0400 @@ -30,6 +30,7 @@ #include "memory/metaspace.hpp" #include "oops/oopHandle.hpp" #include "oops/weakHandle.hpp" +#include "runtime/atomic.hpp" #include "runtime/mutex.hpp" #include "utilities/growableArray.hpp" #include "utilities/macros.hpp" @@ -159,7 +160,7 @@ JFR_ONLY(DEFINE_TRACE_ID_FIELD;) void set_next(ClassLoaderData* next) { _next = next; } - ClassLoaderData* next() const { return _next; } + ClassLoaderData* next() const { return Atomic::load(&_next); } ClassLoaderData(Handle h_class_loader, bool is_unsafe_anonymous); ~ClassLoaderData();
--- a/src/hotspot/share/classfile/classLoaderDataGraph.cpp Thu Jun 06 20:19:03 2019 +0200 +++ b/src/hotspot/share/classfile/classLoaderDataGraph.cpp Thu Jun 06 15:09:17 2019 -0400 @@ -38,6 +38,7 @@ #include "runtime/atomic.hpp" #include "runtime/handles.inline.hpp" #include "runtime/mutex.hpp" +#include "runtime/orderAccess.hpp" #include "runtime/safepoint.hpp" #include "runtime/safepointVerifiers.hpp" #include "utilities/growableArray.hpp" @@ -48,8 +49,17 @@ volatile size_t ClassLoaderDataGraph::_num_instance_classes = 0; void ClassLoaderDataGraph::clear_claimed_marks() { - assert_locked_or_safepoint_weak(ClassLoaderDataGraph_lock); - for (ClassLoaderData* cld = _head; cld != NULL; cld = cld->next()) { + // The claimed marks of the CLDs in the ClassLoaderDataGraph are cleared + // outside a safepoint and without locking the ClassLoaderDataGraph_lock. + // This is required to avoid a deadlock between concurrent GC threads and safepointing. + // + // We need to make sure that the CLD contents are fully visible to the + // reader thread. This is accomplished by acquire/release of the _head, + // and is sufficient. + // + // Any ClassLoaderData added after or during walking the list are prepended to + // _head. Their claim mark need not be handled here. + for (ClassLoaderData* cld = OrderAccess::load_acquire(&_head); cld != NULL; cld = cld->next()) { cld->clear_claim(); } } @@ -169,7 +179,7 @@ } // GC root of class loader data created. -ClassLoaderData* ClassLoaderDataGraph::_head = NULL; +ClassLoaderData* volatile ClassLoaderDataGraph::_head = NULL; ClassLoaderData* ClassLoaderDataGraph::_unloading = NULL; ClassLoaderData* ClassLoaderDataGraph::_saved_unloading = NULL; ClassLoaderData* ClassLoaderDataGraph::_saved_head = NULL; @@ -205,7 +215,7 @@ // First install the new CLD to the Graph. cld->set_next(_head); - _head = cld; + OrderAccess::release_store(&_head, cld); // Next associate with the class_loader. if (!is_unsafe_anonymous) {
--- a/src/hotspot/share/classfile/classLoaderDataGraph.hpp Thu Jun 06 20:19:03 2019 +0200 +++ b/src/hotspot/share/classfile/classLoaderDataGraph.hpp Thu Jun 06 15:09:17 2019 -0400 @@ -41,7 +41,7 @@ friend class VMStructs; private: // All CLDs (except the null CLD) can be reached by walking _head->_next->... - static ClassLoaderData* _head; + static ClassLoaderData* volatile _head; static ClassLoaderData* _unloading; // CMS support. static ClassLoaderData* _saved_head;
--- a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp Thu Jun 06 20:19:03 2019 +0200 +++ b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp Thu Jun 06 15:09:17 2019 -0400 @@ -268,7 +268,6 @@ { G1ConcPhase p(G1ConcurrentPhase::CLEAR_CLAIMED_MARKS, this); - MutexLocker ml(ClassLoaderDataGraph_lock); ClassLoaderDataGraph::clear_claimed_marks(); }
--- a/src/hotspot/share/runtime/vmStructs.cpp Thu Jun 06 20:19:03 2019 +0200 +++ b/src/hotspot/share/runtime/vmStructs.cpp Thu Jun 06 15:09:17 2019 -0400 @@ -526,7 +526,7 @@ nonstatic_field(ClassLoaderData, _is_unsafe_anonymous, bool) \ volatile_nonstatic_field(ClassLoaderData, _dictionary, Dictionary*) \ \ - static_field(ClassLoaderDataGraph, _head, ClassLoaderData*) \ + static_ptr_volatile_field(ClassLoaderDataGraph, _head, ClassLoaderData*) \ \ /**********/ \ /* Arrays */ \