Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8267653: Remove Mutex::_safepoint_check_sometimes #4184

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ void ShenandoahReferenceProcessor::enqueue_references(bool concurrent) {
enqueue_references_locked();
} else {
// Heap_lock protects external pending list
MonitorLocker ml(Heap_lock, Mutex::_no_safepoint_check_flag);
MonitorLocker ml(Heap_lock);
coleenp marked this conversation as resolved.
Show resolved Hide resolved

enqueue_references_locked();

Expand Down
9 changes: 0 additions & 9 deletions src/hotspot/share/runtime/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,6 @@ Mutex::~Mutex() {
os::free(const_cast<char*>(_name));
}

// Only Threads_lock and Heap_lock may be safepoint_check_sometimes.
bool is_sometimes_ok(const char* name) {
return (strcmp(name, "Threads_lock") == 0 || strcmp(name, "Heap_lock") == 0);
}

Mutex::Mutex(int Rank, const char * name, bool allow_vm_block,
SafepointCheckRequired safepoint_check_required) : _owner(NULL) {
assert(os::mutex_init_done(), "Too early!");
Expand All @@ -277,9 +272,6 @@ Mutex::Mutex(int Rank, const char * name, bool allow_vm_block,
_safepoint_check_required = safepoint_check_required;
_skip_rank_check = false;

assert(_safepoint_check_required != _safepoint_check_sometimes || is_sometimes_ok(name),
"Lock has _safepoint_check_sometimes %s", name);

assert(_rank > special || _safepoint_check_required == _safepoint_check_never,
"Special locks or below should never safepoint");
#endif
Expand All @@ -306,7 +298,6 @@ void Mutex::print_on_error(outputStream* st) const {
const char* print_safepoint_check(Mutex::SafepointCheckRequired safepoint_check) {
switch (safepoint_check) {
case Mutex::_safepoint_check_never: return "safepoint_check_never";
case Mutex::_safepoint_check_sometimes: return "safepoint_check_sometimes";
case Mutex::_safepoint_check_always: return "safepoint_check_always";
default: return "";
}
Expand Down
10 changes: 0 additions & 10 deletions src/hotspot/share/runtime/mutex.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ class Mutex : public CHeapObj<mtSynchronizer> {
// _safepoint_check_never, that means that whenever the lock is acquired by a JavaThread
// it will verify that it is done without a safepoint check.


// There are a couple of existing locks that will sometimes have a safepoint check and
// sometimes not when acquired by a JavaThread, but these locks are set up carefully
// to avoid deadlocks. TODO: Fix these locks and remove _safepoint_check_sometimes.

// TODO: Locks that are shared between JavaThreads and NonJavaThreads
// should never encounter a safepoint check while they are held, or else a
// deadlock can occur. We should check this by noting which
Expand All @@ -155,17 +150,12 @@ class Mutex : public CHeapObj<mtSynchronizer> {
enum class SafepointCheckRequired {
_safepoint_check_never, // Mutexes with this value will cause errors
// when acquired by a JavaThread with a safepoint check.
_safepoint_check_sometimes, // A couple of special locks are acquired by JavaThreads sometimes
// with and sometimes without safepoint checks. These
// locks will not produce errors when locked.
_safepoint_check_always // Mutexes with this value will cause errors
// when acquired by a JavaThread without a safepoint check.
};
// Bring the enumerator names into class scope.
static const SafepointCheckRequired _safepoint_check_never =
SafepointCheckRequired::_safepoint_check_never;
static const SafepointCheckRequired _safepoint_check_sometimes =
SafepointCheckRequired::_safepoint_check_sometimes;
static const SafepointCheckRequired _safepoint_check_always =
SafepointCheckRequired::_safepoint_check_always;

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/mutexLocker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ void mutex_init() {
def(JNICritical_lock , PaddedMonitor, nonleaf, true, _safepoint_check_always); // used for JNI critical regions
def(AdapterHandlerLibrary_lock , PaddedMutex , nonleaf, true, _safepoint_check_always);

def(Heap_lock , PaddedMonitor, nonleaf+1, false, _safepoint_check_sometimes); // Doesn't safepoint check during termination.
def(Heap_lock , PaddedMonitor, nonleaf+1, false, _safepoint_check_always); // Doesn't safepoint check during termination.
def(JfieldIdCreation_lock , PaddedMutex , nonleaf+1, true, _safepoint_check_always); // jfieldID, Used in VM_Operation

def(CompiledIC_lock , PaddedMutex , nonleaf+2, false, _safepoint_check_never); // locks VtableStubs_lock, InlineCacheBuffer_lock
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/share/runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3416,8 +3416,12 @@ void Threads::destroy_vm() {
// to prevent this. The GC vm_operations will not be able to
// queue until after the vm thread is dead. After this point,
// we'll never emerge out of the safepoint before the VM exits.
// Assert that the thread is terminated so that acquiring the
// Heap_lock doesn't cause the terminated thread to participate in
// the safepoint protocol.

MutexLocker ml(Heap_lock, Mutex::_no_safepoint_check_flag);
assert(thread->is_terminated(), "must be terminated here");
MutexLocker ml(Heap_lock);
coleenp marked this conversation as resolved.
Show resolved Hide resolved

VMThread::wait_for_vm_thread_exit();
assert(SafepointSynchronize::is_at_safepoint(), "VM thread should exit at Safepoint");
Expand Down