Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Commit

Permalink
8278389: SuspendibleThreadSet::_suspend_all should be volatile/atomic
Browse files Browse the repository at this point in the history
Reviewed-by: ayang, mli
  • Loading branch information
Thomas Schatzl committed Dec 16, 2021
1 parent e82310f commit aec1b03
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
26 changes: 13 additions & 13 deletions src/hotspot/share/gc/shared/suspendibleThreadSet.cpp
Expand Up @@ -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;

Expand All @@ -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++;
Expand All @@ -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();
}
Expand All @@ -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) {
Expand All @@ -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");
Expand All @@ -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;
}
Expand All @@ -120,16 +120,16 @@ 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
}

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();
}
8 changes: 6 additions & 2 deletions src/hotspot/share/gc/shared/suspendibleThreadSet.hpp
Expand Up @@ -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.
Expand All @@ -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();
Expand All @@ -53,9 +55,11 @@ class SuspendibleThreadSet : public AllStatic {
// 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();
Expand Down

1 comment on commit aec1b03

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.