8224151: Test vmTestbase/nsk/sysdict/vm/stress/chain/chain007/chain007.java might hang with release binaries
authorcoleenp
Thu, 06 Jun 2019 15:09:17 -0400
changeset 55268 c5fe45d1281d
parent 55267 eabe64456156
child 55269 098fa5ddb308
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
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
--- 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 */                                                                                                                       \