Skip to content

Commit

Permalink
8273300: Check Mutex ranking during a safepoint
Browse files Browse the repository at this point in the history
Reviewed-by: eosterlund, dholmes, pchilanomate
  • Loading branch information
coleenp committed Sep 16, 2021
1 parent c86e24d commit 5e4d09c
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/classLoaderData.cpp
Expand Up @@ -133,7 +133,7 @@ void ClassLoaderData::initialize_name(Handle class_loader) {

ClassLoaderData::ClassLoaderData(Handle h_class_loader, bool has_class_mirror_holder) :
_metaspace(NULL),
_metaspace_lock(new Mutex(Mutex::leaf+1, "Metaspace allocation lock",
_metaspace_lock(new Mutex(Mutex::leaf-2, "MetaspaceAllocation_lock",
Mutex::_safepoint_check_never)),
_unloading(false), _has_class_mirror_holder(has_class_mirror_holder),
_modified_oops(true),
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
Expand Up @@ -167,7 +167,7 @@ class G1RegionsSmallerThanCommitSizeMapper : public G1RegionToSpaceMapper {
MEMFLAGS type) :
G1RegionToSpaceMapper(rs, actual_size, page_size, alloc_granularity, commit_factor, type),
_regions_per_page((page_size * commit_factor) / alloc_granularity),
_lock(Mutex::leaf, "G1 mapper lock", Mutex::_safepoint_check_never) {
_lock(Mutex::service-3, "G1Mapper_lock", Mutex::_safepoint_check_never) {

guarantee((page_size * commit_factor) >= alloc_granularity, "allocation granularity smaller than commit granularity");
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/heapRegion.cpp
Expand Up @@ -233,7 +233,7 @@ HeapRegion::HeapRegion(uint hrm_index,
_top(NULL),
_compaction_top(NULL),
_bot_part(bot, this),
_par_alloc_lock(Mutex::leaf, "HeapRegion par alloc lock", Mutex::_safepoint_check_always, true),
_par_alloc_lock(Mutex::service-2, "HeapRegionParAlloc_lock", Mutex::_safepoint_check_never),
_pre_dummy_top(NULL),
_rem_set(NULL),
_hrm_index(hrm_index),
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/heapRegion.inline.hpp
Expand Up @@ -104,7 +104,7 @@ inline HeapWord* HeapRegion::par_allocate(size_t word_size) {
inline HeapWord* HeapRegion::par_allocate(size_t min_word_size,
size_t desired_word_size,
size_t* actual_size) {
MutexLocker x(&_par_alloc_lock);
MutexLocker x(&_par_alloc_lock, Mutex::_no_safepoint_check_flag);
return allocate(min_word_size, desired_word_size, actual_size);
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/heapRegionRemSet.cpp
Expand Up @@ -47,7 +47,7 @@ const char* HeapRegionRemSet::_short_state_strings[] = {"UNTRA", "UPDAT", "CMPL

HeapRegionRemSet::HeapRegionRemSet(HeapRegion* hr,
G1CardSetConfiguration* config) :
_m(Mutex::leaf + 1, FormatBuffer<128>("HeapRegionRemSet lock #%u", hr->hrm_index()), Monitor::_safepoint_check_never),
_m(Mutex::service - 1, FormatBuffer<128>("HeapRegionRemSet#%u_lock", hr->hrm_index()), Monitor::_safepoint_check_never),
_code_roots(),
_card_set_mm(config, G1CardSetFreePool::free_list_pool()),
_card_set(config, &_card_set_mm),
Expand Down
2 changes: 0 additions & 2 deletions src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
Expand Up @@ -355,14 +355,12 @@ ShenandoahCodeRootsIterator::ShenandoahCodeRootsIterator() :
_par_iterator(CodeCache::heaps()),
_table_snapshot(NULL) {
assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint");
CodeCache_lock->lock_without_safepoint_check();
_table_snapshot = ShenandoahCodeRoots::table()->snapshot_for_iteration();
}

ShenandoahCodeRootsIterator::~ShenandoahCodeRootsIterator() {
ShenandoahCodeRoots::table()->finish_iteration(_table_snapshot);
_table_snapshot = NULL;
CodeCache_lock->unlock();
}

void ShenandoahCodeRootsIterator::possibly_parallel_blobs_do(CodeBlobClosure *f) {
Expand Down
Expand Up @@ -100,7 +100,6 @@ ShenandoahReentrantLock* ShenandoahNMethod::lock_for_nmethod(nmethod* nm) {
}

bool ShenandoahNMethodTable::iteration_in_progress() const {
shenandoah_assert_locked_or_safepoint(CodeCache_lock);
return _itr_cnt > 0;
}

Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/memory/universe.cpp
Expand Up @@ -1106,7 +1106,6 @@ void Universe::verify(VerifyOption option, const char* prefix) {
StringTable::verify();
}
if (should_verify_subset(Verify_CodeCache)) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
log_debug(gc, verify)("CodeCache");
CodeCache::verify();
}
Expand Down
26 changes: 11 additions & 15 deletions src/hotspot/share/runtime/mutex.cpp
Expand Up @@ -289,14 +289,14 @@ Mutex::Mutex(int Rank, const char * name, SafepointCheckRequired safepoint_check
"higher than nonleaf should safepoint %s", name);

assert(_rank > special || _safepoint_check_required == _safepoint_check_never,
"Special locks or below should never safepoint");
"Special locks or below should never safepoint: %s", name);

// The allow_vm_block also includes allowing other non-Java threads to block or
// allowing Java threads to block in native.
assert(_safepoint_check_required == _safepoint_check_always || _allow_vm_block,
"Safepoint check never locks should always allow the vm to block");
"Safepoint check never locks should always allow the vm to block: %s", name);

assert(_rank >= 0, "Bad lock rank");
assert(_rank >= 0, "Bad lock rank: %s", name);
#endif
}

Expand Down Expand Up @@ -372,31 +372,27 @@ Mutex* Mutex::get_least_ranked_lock_besides_this(Mutex* locks) {
void Mutex::check_rank(Thread* thread) {
Mutex* locks_owned = thread->owned_locks();

if (!SafepointSynchronize::is_at_safepoint()) {
// We expect the locks already acquired to be in increasing rank order,
// modulo locks acquired in try_lock_without_rank_check()
for (Mutex* tmp = locks_owned; tmp != NULL; tmp = tmp->next()) {
if (tmp->next() != NULL) {
assert(tmp->rank() < tmp->next()->rank()
|| tmp->skip_rank_check(), "mutex rank anomaly?");
}
// We expect the locks already acquired to be in increasing rank order,
// modulo locks acquired in try_lock_without_rank_check()
for (Mutex* tmp = locks_owned; tmp != NULL; tmp = tmp->next()) {
if (tmp->next() != NULL) {
assert(tmp->rank() < tmp->next()->rank()
|| tmp->skip_rank_check(), "mutex rank anomaly?");
}
}

bool check_can_be_skipped = SafepointSynchronize::is_at_safepoint();
if (owned_by_self()) {
// wait() case
Mutex* least = get_least_ranked_lock_besides_this(locks_owned);
// We enforce not holding locks of rank special or lower while waiting.
// Also "this" should be the monitor with lowest rank owned by this thread.
if (least != NULL && (least->rank() <= special ||
(least->rank() <= this->rank() && !check_can_be_skipped))) {
if (least != NULL && (least->rank() <= special || least->rank() <= this->rank())) {
assert(false, "Attempting to wait on monitor %s/%d while holding lock %s/%d -- "
"possible deadlock. %s", name(), rank(), least->name(), least->rank(),
least->rank() <= this->rank() ? "Should wait on the least ranked monitor from "
"all owned locks." : "Should not block(wait) while holding a lock of rank special.");
}
} else if (!check_can_be_skipped) {
} else {
// lock()/lock_without_safepoint_check()/try_lock() case
Mutex* least = get_least_ranked_lock(locks_owned);
// Deadlock prevention rules require us to acquire Mutexes only in
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/runtime/mutex.hpp
Expand Up @@ -45,12 +45,12 @@ class Mutex : public CHeapObj<mtSynchronizer> {
// Special low level locks are given names and ranges avoid overlap.
enum lock_types {
event,
service = event + 3,
service = event + 6,
stackwatermark = service + 3,
tty = stackwatermark + 3,
special = tty + 3,
oopstorage = special + 3,
leaf = oopstorage + 2,
leaf = oopstorage + 10,
safepoint = leaf + 10,
barrier = safepoint + 1,
nonleaf = barrier + 1,
Expand Down
22 changes: 11 additions & 11 deletions src/hotspot/share/runtime/mutexLocker.cpp
Expand Up @@ -204,18 +204,18 @@ void assert_locked_or_safepoint_or_handshake(const Mutex* lock, const JavaThread
void mutex_init() {
def(tty_lock , PaddedMutex , tty, true, _safepoint_check_never); // allow to lock in VM

def(CGC_lock , PaddedMonitor, special, true, _safepoint_check_never); // coordinate between fore- and background GC
def(STS_lock , PaddedMonitor, leaf, true, _safepoint_check_never);

if (UseG1GC) {
def(CGC_lock , PaddedMonitor, leaf, true, _safepoint_check_never);
def(G1OldGCCount_lock , PaddedMonitor, leaf, true, _safepoint_check_always);

def(G1DetachedRefinementStats_lock, PaddedMutex, leaf , true, _safepoint_check_never);
def(G1DetachedRefinementStats_lock, PaddedMutex, leaf-2, true, _safepoint_check_never);

def(FreeList_lock , PaddedMutex , leaf , true, _safepoint_check_never);
def(OldSets_lock , PaddedMutex , leaf , true, _safepoint_check_never);
def(Uncommit_lock , PaddedMutex , leaf + 1 , true, _safepoint_check_never);
def(RootRegionScan_lock , PaddedMonitor, leaf , true, _safepoint_check_never);
def(FreeList_lock , PaddedMutex , service-1, true, _safepoint_check_never);
def(OldSets_lock , PaddedMutex , leaf, true, _safepoint_check_never);
def(Uncommit_lock , PaddedMutex , service-2, true, _safepoint_check_never);
def(RootRegionScan_lock , PaddedMonitor, leaf-1, true, _safepoint_check_never);

def(MarkStackFreeList_lock , PaddedMutex , leaf , true, _safepoint_check_never);
def(MarkStackChunkList_lock , PaddedMutex , leaf , true, _safepoint_check_never);
Expand All @@ -230,7 +230,7 @@ void mutex_init() {
def(RawMonitor_lock , PaddedMutex , special, true, _safepoint_check_never);
def(OopMapCacheAlloc_lock , PaddedMutex , leaf, true, _safepoint_check_always); // used for oop_map_cache allocation.

def(Metaspace_lock , PaddedMutex , leaf-1, true, _safepoint_check_never);
def(Metaspace_lock , PaddedMutex , leaf-3, true, _safepoint_check_never);
def(ClassLoaderDataGraph_lock , PaddedMutex , nonleaf, false, _safepoint_check_always);

def(Patching_lock , PaddedMutex , special, true, _safepoint_check_never); // used for safepointing and code patching.
Expand Down Expand Up @@ -265,14 +265,14 @@ void mutex_init() {
def(PerfDataManager_lock , PaddedMutex , leaf, true, _safepoint_check_always); // used for synchronized access to PerfDataManager resources

def(Threads_lock , PaddedMonitor, barrier, true, _safepoint_check_always); // Used for safepoint protocol.
def(NonJavaThreadsList_lock , PaddedMutex, barrier, true, _safepoint_check_never);
def(NonJavaThreadsList_lock , PaddedMutex, leaf-1, true, _safepoint_check_never);
def(NonJavaThreadsListSync_lock , PaddedMutex, leaf, true, _safepoint_check_never);

def(VMOperation_lock , PaddedMonitor, nonleaf, true, _safepoint_check_always); // VM_thread allowed to block on these
def(RetData_lock , PaddedMutex , nonleaf, false, _safepoint_check_always);
def(Terminator_lock , PaddedMonitor, nonleaf, true, _safepoint_check_always);
def(InitCompleted_lock , PaddedMonitor, leaf, true, _safepoint_check_never);
def(VtableStubs_lock , PaddedMutex , leaf, true, _safepoint_check_never);
def(VtableStubs_lock , PaddedMutex , leaf-2, true, _safepoint_check_never);
def(Notify_lock , PaddedMonitor, nonleaf, true, _safepoint_check_always);
def(JNICritical_lock , PaddedMonitor, nonleaf, true, _safepoint_check_always); // used for JNI critical regions
def(AdapterHandlerLibrary_lock , PaddedMutex , nonleaf, true, _safepoint_check_always);
Expand Down Expand Up @@ -300,7 +300,7 @@ void mutex_init() {
def(PeriodicTask_lock , PaddedMonitor, nonleaf+5, true, _safepoint_check_always);
def(RedefineClasses_lock , PaddedMonitor, nonleaf+5, true, _safepoint_check_always);
def(Verify_lock , PaddedMutex, nonleaf+5, true, _safepoint_check_always);
def(Zip_lock , PaddedMonitor, leaf, true, _safepoint_check_never);
def(Zip_lock , PaddedMonitor, leaf-2, true, _safepoint_check_never);

if (WhiteBoxAPI) {
def(Compilation_lock , PaddedMonitor, leaf, true, _safepoint_check_never);
Expand Down Expand Up @@ -330,7 +330,7 @@ void mutex_init() {
#if INCLUDE_JVMTI
def(CDSClassFileStream_lock , PaddedMutex , max_nonleaf, false, _safepoint_check_always);
#endif
def(DumpTimeTable_lock , PaddedMutex , leaf - 1, true, _safepoint_check_never);
def(DumpTimeTable_lock , PaddedMutex , leaf-1, true, _safepoint_check_never);
def(CDSLambda_lock , PaddedMutex , leaf, true, _safepoint_check_never);
def(DumpRegion_lock , PaddedMutex , leaf, true, _safepoint_check_never);
def(ClassListFile_lock , PaddedMutex , leaf, true, _safepoint_check_never);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/utilities/concurrentHashTable.inline.hpp
Expand Up @@ -1014,7 +1014,7 @@ inline ConcurrentHashTable<CONFIG, F>::
{
_stats_rate = TableRateStatistics();
_resize_lock =
new Mutex(Mutex::leaf, "ConcurrentHashTable",
new Mutex(Mutex::leaf-2, "ConcurrentHashTableResize_lock",
Mutex::_safepoint_check_never);
_table = new InternalTable(log2size);
assert(log2size_limit >= log2size, "bad ergo");
Expand Down

1 comment on commit 5e4d09c

@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.