From aec1b039b35b73db17c943cdd86949a92e64fcb6 Mon Sep 17 00:00:00 2001
From: Thomas Schatzl <tschatzl@openjdk.org>
Date: Thu, 16 Dec 2021 12:29:57 +0000
Subject: [PATCH] 8278389: SuspendibleThreadSet::_suspend_all should be
volatile/atomic
Reviewed-by: ayang, mli
---
.../share/gc/shared/suspendibleThreadSet.cpp | 26 +++++++++----------
.../share/gc/shared/suspendibleThreadSet.hpp | 8 ++++--
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/src/hotspot/share/gc/shared/suspendibleThreadSet.cpp b/src/hotspot/share/gc/shared/suspendibleThreadSet.cpp
index e91b1b49802..2636cc92061 100644
--- a/src/hotspot/share/gc/shared/suspendibleThreadSet.cpp
+++ b/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();
}
diff --git a/src/hotspot/share/gc/shared/suspendibleThreadSet.hpp b/src/hotspot/share/gc/shared/suspendibleThreadSet.hpp
index 1e47c3b57a8..37d27f3e9ed 100644
--- a/src/hotspot/share/gc/shared/suspendibleThreadSet.hpp
+++ b/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();