8278389: SuspendibleThreadSet::_suspend_all should be volatile/atomic

Reviewed-by: ayang, mli

src/hotspot/share/gc/shared/suspendibleThreadSet.cpp

@@ -29,10 +29,10 @@
 #include "runtime/semaphore.hpp"
 #include "runtime/thread.inline.hpp"
 
-uint   SuspendibleThreadSet::_nthreads          = 0;
-uint   SuspendibleThreadSet::_nthreads_stopped  = 0;
-bool   SuspendibleThreadSet::_suspend_all       = false;
-double SuspendibleThreadSet::_suspend_all_start = 0.0;
+uint   SuspendibleThreadSet::_nthreads            = 0;
+uint   SuspendibleThreadSet::_nthreads_stopped    = 0;
+volatile bool SuspendibleThreadSet::_suspend_all  = false;
+double SuspendibleThreadSet::_suspend_all_start   = 0.0;
 
 static Semaphore* _synchronize_wakeup = NULL;
 
@@ -50,7 +50,7 @@ bool SuspendibleThreadSet::is_synchronized() {
 void SuspendibleThreadSet::join() {
   assert(!Thread::current()->is_suspendible_thread(), "Thread already joined");
   MonitorLocker ml(STS_lock, Mutex::_no_safepoint_check_flag);
-  while (_suspend_all) {
+  while (suspend_all()) {
     ml.wait();
   }
   _nthreads++;
@@ -63,7 +63,7 @@ void SuspendibleThreadSet::leave() {
   assert(_nthreads > 0, "Invalid");
   DEBUG_ONLY(Thread::current()->clear_suspendible_thread();)
   _nthreads--;
-  if (_suspend_all && is_synchronized()) {
+  if (suspend_all() && is_synchronized()) {
     // This leave completes a request, so inform the requestor.
     _synchronize_wakeup->signal();
   }
@@ -72,7 +72,7 @@ void SuspendibleThreadSet::leave() {
 void SuspendibleThreadSet::yield() {
   assert(Thread::current()->is_suspendible_thread(), "Must have joined");
   MonitorLocker ml(STS_lock, Mutex::_no_safepoint_check_flag);
-  if (_suspend_all) {
+  if (suspend_all()) {
     _nthreads_stopped++;
     if (is_synchronized()) {
       if (ConcGCYieldTimeout > 0) {
@@ -82,7 +82,7 @@ void SuspendibleThreadSet::yield() {
       // This yield completes the request, so inform the requestor.
       _synchronize_wakeup->signal();
     }
-    while (_suspend_all) {
+    while (suspend_all()) {
       ml.wait();
     }
     assert(_nthreads_stopped > 0, "Invalid");
@@ -97,8 +97,8 @@ void SuspendibleThreadSet::synchronize() {
   }
   {
     MonitorLocker ml(STS_lock, Mutex::_no_safepoint_check_flag);
-    assert(!_suspend_all, "Only one at a time");
-    _suspend_all = true;
+    assert(!suspend_all(), "Only one at a time");
+    Atomic::store(&_suspend_all, true);
     if (is_synchronized()) {
       return;
     }
@@ -120,7 +120,7 @@ void SuspendibleThreadSet::synchronize() {
 
 #ifdef ASSERT
   MonitorLocker ml(STS_lock, Mutex::_no_safepoint_check_flag);
-  assert(_suspend_all, "STS not synchronizing");
+  assert(suspend_all(), "STS not synchronizing");
   assert(is_synchronized(), "STS not synchronized");
 #endif
 }
@@ -128,8 +128,8 @@ void SuspendibleThreadSet::synchronize() {
 void SuspendibleThreadSet::desynchronize() {
   assert(Thread::current()->is_VM_thread(), "Must be the VM thread");
   MonitorLocker ml(STS_lock, Mutex::_no_safepoint_check_flag);
-  assert(_suspend_all, "STS not synchronizing");
+  assert(suspend_all(), "STS not synchronizing");
   assert(is_synchronized(), "STS not synchronized");
-  _suspend_all = false;
+  Atomic::store(&_suspend_all, false);
   ml.notify_all();
 }

src/hotspot/share/gc/shared/suspendibleThreadSet.hpp

@@ -26,6 +26,7 @@
 #define SHARE_GC_SHARED_SUSPENDIBLETHREADSET_HPP
 
 #include "memory/allocation.hpp"
+#include "runtime/atomic.hpp"
 
 // A SuspendibleThreadSet is a set of threads that can be suspended.
 // A thread can join and later leave the set, and periodically yield.
@@ -40,9 +41,10 @@ class SuspendibleThreadSet : public AllStatic {
   friend class SuspendibleThreadSetLeaver;
 
 private:
+  static volatile bool _suspend_all;
+
   static uint   _nthreads;
   static uint   _nthreads_stopped;
-  static bool   _suspend_all;
   static double _suspend_all_start;
 
   static bool is_synchronized();
@@ -53,9 +55,11 @@ private:
   // Removes the current thread from the set.
   static void leave();
 
+  static bool suspend_all() { return Atomic::load(&_suspend_all); }
+
 public:
   // Returns true if an suspension is in progress.
-  static bool should_yield() { return _suspend_all; }
+  static bool should_yield() { return suspend_all(); }
 
   // Suspends the current thread if a suspension is in progress.
   static void yield();