# HG changeset patch # User coleenp # Date 1556715398 14400 # Node ID f03d5a0930934496fc719150e56ebc900f2343cb # Parent afce4a27f2c2d5044593674fa8542ca9c11fa7bd 8074355: make MutexLocker smarter about non-JavaThreads Summary: Use safepoint_check_always/safepoint_check_never instead of safepoint_check_sometimes for locks that are taken by JavaThreads and non-JavaThreads Reviewed-by: dholmes, rehn, dcubed, lmesnik diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp --- a/src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/gc/cms/compactibleFreeListSpace.cpp Wed May 01 08:56:38 2019 -0400 @@ -331,11 +331,11 @@ // Note: this requires that CFLspace c'tors // are called serially in the order in which the locks are // are acquired in the program text. This is true today. - _freelistLock(_lockRank--, "CompactibleFreeListSpace._lock", true, - Monitor::_safepoint_check_sometimes), + _freelistLock(_lockRank--, "CompactibleFreeListSpace_lock", true, + Monitor::_safepoint_check_never), _preconsumptionDirtyCardClosure(NULL), _parDictionaryAllocLock(Mutex::leaf - 1, // == rank(ExpandHeap_lock) - 1 - "CompactibleFreeListSpace._dict_par_lock", true, + "CompactibleFreeListSpace_dict_par_lock", true, Monitor::_safepoint_check_never) { assert(sizeof(FreeChunk) / BytesPerWord <= MinChunkSize, @@ -366,7 +366,7 @@ // Initialize locks for parallel case. for (size_t i = IndexSetStart; i < IndexSetSize; i += IndexSetStride) { _indexedFreeListParLocks[i] = new Mutex(Mutex::leaf - 1, // == ExpandHeap_lock - 1 - "a freelist par lock", true, Mutex::_safepoint_check_sometimes); + "a freelist par lock", true, Mutex::_safepoint_check_never); DEBUG_ONLY( _indexedFreeList[i].set_protecting_lock(_indexedFreeListParLocks[i]); ) @@ -2042,7 +2042,7 @@ if (rem_sz < SmallForDictionary) { // The freeList lock is held, but multiple GC task threads might be executing in parallel. bool is_par = Thread::current()->is_GC_task_thread(); - if (is_par) _indexedFreeListParLocks[rem_sz]->lock(); + if (is_par) _indexedFreeListParLocks[rem_sz]->lock_without_safepoint_check(); returnChunkToFreeList(ffc); split(size, rem_sz); if (is_par) _indexedFreeListParLocks[rem_sz]->unlock(); diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp --- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Wed May 01 08:56:38 2019 -0400 @@ -491,7 +491,7 @@ _stats(cmsGen), _eden_chunk_lock(new Mutex(Mutex::leaf + 1, "CMS_eden_chunk_lock", true, //verify that this lock should be acquired with safepoint check. - Monitor::_safepoint_check_sometimes)), + Monitor::_safepoint_check_never)), _eden_chunk_array(NULL), // may be set in ctor body _eden_chunk_index(0), // -- ditto -- _eden_chunk_capacity(0), // -- ditto -- @@ -5643,7 +5643,7 @@ _shifter(shifter), _bm(), _lock(mutex_rank >= 0 ? new Mutex(mutex_rank, mutex_name, true, - Monitor::_safepoint_check_sometimes) : NULL) + Monitor::_safepoint_check_never) : NULL) { _bmStartWord = 0; _bmWordSize = 0; @@ -7526,7 +7526,7 @@ } ConcurrentMarkSweepThread::synchronize(true); - _freelistLock->lock(); + _freelistLock->lock_without_safepoint_check(); _bitMap->lock()->lock_without_safepoint_check(); _collector->startTimer(); } diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/gc/cms/yieldingWorkgroup.cpp --- a/src/hotspot/share/gc/cms/yieldingWorkgroup.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/gc/cms/yieldingWorkgroup.cpp Wed May 01 08:56:38 2019 -0400 @@ -43,7 +43,7 @@ _monitor = new Monitor(/* priority */ Mutex::leaf, /* name */ "WorkGroup monitor", /* allow_vm_block */ are_GC_task_threads, - Monitor::_safepoint_check_sometimes); + Monitor::_safepoint_check_never); assert(monitor() != NULL, "Failed to allocate monitor"); } diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/gc/shared/oopStorage.cpp --- a/src/hotspot/share/gc/shared/oopStorage.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/gc/shared/oopStorage.cpp Wed May 01 08:56:38 2019 -0400 @@ -765,10 +765,10 @@ "%s: active_mutex must have lower rank than allocation_mutex", _name); assert(Service_lock->rank() < _active_mutex->rank(), "%s: active_mutex must have higher rank than Service_lock", _name); - assert(_active_mutex->_safepoint_check_required != Mutex::_safepoint_check_always, - "%s: active mutex requires safepoint check", _name); - assert(_allocation_mutex->_safepoint_check_required != Mutex::_safepoint_check_always, - "%s: allocation mutex requires safepoint check", _name); + assert(_active_mutex->_safepoint_check_required == Mutex::_safepoint_check_never, + "%s: active mutex requires never safepoint check", _name); + assert(_allocation_mutex->_safepoint_check_required == Mutex::_safepoint_check_never, + "%s: allocation mutex requires never safepoint check", _name); } void OopStorage::delete_empty_block(const Block& block) { diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp --- a/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp Wed May 01 08:56:38 2019 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -136,7 +136,7 @@ public: StackTraceWrite(JfrStackTraceRepository& stack_trace_repo, JfrCheckpointWriter& writer) : _stack_trace_repo(stack_trace_repo), _writer(writer), _count(0) { - JfrStacktrace_lock->lock(); + JfrStacktrace_lock->lock_without_safepoint_check(); } ~StackTraceWrite() { assert(JfrStacktrace_lock->owned_by_self(), "invariant"); diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/runtime/mutex.cpp --- a/src/hotspot/share/runtime/mutex.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/runtime/mutex.cpp Wed May 01 08:56:38 2019 -0400 @@ -32,11 +32,23 @@ #include "utilities/events.hpp" #include "utilities/macros.hpp" +#ifdef ASSERT +void Monitor::check_safepoint_state(Thread* thread, bool do_safepoint_check) { + // If the JavaThread checks for safepoint, verify that the lock wasn't created with safepoint_check_never. + SafepointCheckRequired not_allowed = do_safepoint_check ? Monitor::_safepoint_check_never : + Monitor::_safepoint_check_always; + assert(!thread->is_Java_thread() || _safepoint_check_required != not_allowed, + "This lock should %s have a safepoint check for Java threads: %s", + _safepoint_check_required ? "always" : "never", name()); + + // If defined with safepoint_check_never, a NonJavaThread should never ask to safepoint check either. + assert(thread->is_Java_thread() || !do_safepoint_check || _safepoint_check_required != Monitor::_safepoint_check_never, + "NonJavaThread should not check for safepoint"); +} +#endif // ASSERT void Monitor::lock(Thread * self) { - // Ensure that the Monitor requires/allows safepoint checks. - assert(_safepoint_check_required != Monitor::_safepoint_check_never, - "This lock should never have a safepoint check: %s", name()); + check_safepoint_state(self, true); #ifdef CHECK_UNHANDLED_OOPS // Clear unhandled oops in JavaThreads so we get a crash right away. @@ -91,9 +103,7 @@ // in the wrong way this can lead to a deadlock with the safepoint code. void Monitor::lock_without_safepoint_check(Thread * self) { - // Ensure that the Monitor does not require safepoint checks. - assert(_safepoint_check_required != Monitor::_safepoint_check_always, - "This lock should always have a safepoint check: %s", name()); + check_safepoint_state(self, false); assert(_owner != self, "invariant"); _lock.lock(); assert_owner(NULL); @@ -154,14 +164,12 @@ #endif // ASSERT bool Monitor::wait_without_safepoint_check(long timeout) { - // Make sure safepoint checking is used properly. - assert(_safepoint_check_required != Monitor::_safepoint_check_always, - "This lock should always have a safepoint check: %s", name()); + Thread* const self = Thread::current(); + check_safepoint_state(self, false); // timeout is in milliseconds - with zero meaning never timeout assert(timeout >= 0, "negative timeout"); - Thread * const self = Thread::current(); assert_owner(self); assert_wait_lock_state(self); @@ -174,14 +182,12 @@ } bool Monitor::wait(long timeout, bool as_suspend_equivalent) { - // Make sure safepoint checking is used properly. - assert(_safepoint_check_required != Monitor::_safepoint_check_never, - "This lock should never have a safepoint check: %s", name()); + Thread* const self = Thread::current(); + check_safepoint_state(self, true); // timeout is in milliseconds - with zero meaning never timeout assert(timeout >= 0, "negative timeout"); - Thread* const self = Thread::current(); assert_owner(self); // Safepoint checking logically implies java_thread @@ -273,6 +279,12 @@ ClearMonitor(this); } + +// Only Threads_lock, Heap_lock and SR_lock may be safepoint_check_sometimes. +bool is_sometimes_ok(const char* name) { + return (strcmp(name, "Threads_lock") == 0 || strcmp(name, "Heap_lock") == 0 || strcmp(name, "SR_lock") == 0); +} + Monitor::Monitor(int Rank, const char * name, bool allow_vm_block, SafepointCheckRequired safepoint_check_required) { assert(os::mutex_init_done(), "Too early!"); @@ -281,6 +293,9 @@ _allow_vm_block = allow_vm_block; _rank = Rank; NOT_PRODUCT(_safepoint_check_required = safepoint_check_required;) + + assert(_safepoint_check_required != Monitor::_safepoint_check_sometimes || is_sometimes_ok(name), + "Lock has _safepoint_check_sometimes %s", name); #endif } @@ -291,6 +306,9 @@ _allow_vm_block = allow_vm_block; _rank = Rank; NOT_PRODUCT(_safepoint_check_required = safepoint_check_required;) + + assert(_safepoint_check_required != Monitor::_safepoint_check_sometimes || is_sometimes_ok(name), + "Lock has _safepoint_check_sometimes %s", name); #endif } diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/runtime/mutex.hpp --- a/src/hotspot/share/runtime/mutex.hpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/runtime/mutex.hpp Wed May 01 08:56:38 2019 -0400 @@ -92,6 +92,7 @@ void set_owner_implementation(Thread* owner) PRODUCT_RETURN; void check_prelock_state (Thread* thread, bool safepoint_check) PRODUCT_RETURN; void check_block_state (Thread* thread) PRODUCT_RETURN; + void check_safepoint_state (Thread* thread, bool safepoint_check) NOT_DEBUG_RETURN; void assert_owner (Thread* expected) NOT_DEBUG_RETURN; void assert_wait_lock_state (Thread* self) NOT_DEBUG_RETURN; @@ -101,28 +102,39 @@ _as_suspend_equivalent_flag = true }; + // Locks can be acquired with or without a safepoint check. NonJavaThreads do not follow + // the safepoint protocol when acquiring locks. + + // Each lock can be acquired by only JavaThreads, only NonJavaThreads, or shared between + // Java and NonJavaThreads. When the lock is initialized with _safepoint_check_always, + // that means that whenever the lock is acquired by a JavaThread, it will verify that + // it is done with a safepoint check. In corollary, when the lock is initialized with + // _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 + // locks are shared, and walk held locks during safepoint checking. + enum SafepointCheckFlag { _safepoint_check_flag, _no_safepoint_check_flag }; - // Locks can be acquired with or without safepoint check. - // Monitor::lock and Monitor::lock_without_safepoint_check - // checks these flags when acquiring a lock to ensure - // consistent checking for each lock. - // A few existing locks will sometimes have a safepoint check and - // sometimes not, but these locks are set up in such a way to avoid deadlocks. - // Note: monitors that may be shared between JavaThreads and the VMThread - // should never encounter a safepoint check whilst they are held, else a - // deadlock with the VMThread can occur. enum SafepointCheckRequired { _safepoint_check_never, // Monitors with this value will cause errors - // when acquired with a safepoint check. - _safepoint_check_sometimes, // Certain locks are called sometimes with and - // sometimes without safepoint checks. These + // 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 // Causes error if locked without a safepoint - // check. + _safepoint_check_always // Monitors with this value will cause errors + // when acquired by a JavaThread without a safepoint check. }; NOT_PRODUCT(SafepointCheckRequired _safepoint_check_required;) @@ -159,7 +171,7 @@ // Lock without safepoint check. Should ONLY be used by safepoint code and other code // that is guaranteed not to block while running inside the VM. void lock_without_safepoint_check(); - void lock_without_safepoint_check (Thread * Self) ; + void lock_without_safepoint_check(Thread* self); // Current owner - not not MT-safe. Can only be used to guarantee that // the current running thread owns the lock diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/runtime/mutexLocker.cpp --- a/src/hotspot/share/runtime/mutexLocker.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/runtime/mutexLocker.cpp Wed May 01 08:56:38 2019 -0400 @@ -241,53 +241,53 @@ def(StringDedupQueue_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_never); def(StringDedupTable_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); } - def(ParGCRareEvent_lock , PaddedMutex , leaf , true, Monitor::_safepoint_check_sometimes); + def(ParGCRareEvent_lock , PaddedMutex , leaf , true, Monitor::_safepoint_check_always); def(DerivedPointerTableGC_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); - def(CGCPhaseManager_lock , PaddedMonitor, leaf, false, Monitor::_safepoint_check_sometimes); + def(CGCPhaseManager_lock , PaddedMonitor, leaf, false, Monitor::_safepoint_check_always); def(CodeCache_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); def(RawMonitor_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); - def(OopMapCacheAlloc_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for oop_map_cache allocation. + def(OopMapCacheAlloc_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for oop_map_cache allocation. def(MetaspaceExpand_lock , PaddedMutex , leaf-1, true, Monitor::_safepoint_check_never); def(ClassLoaderDataGraph_lock , PaddedMutex , nonleaf, true, Monitor::_safepoint_check_always); def(Patching_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); // used for safepointing and code patching. def(Service_lock , PaddedMonitor, special, true, Monitor::_safepoint_check_never); // used for service thread operations - def(JmethodIdCreation_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for creating jmethodIDs. + def(JmethodIdCreation_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for creating jmethodIDs. - def(SystemDictionary_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_always); // lookups done by VM thread + def(SystemDictionary_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_always); def(ProtectionDomainSet_lock , PaddedMutex , leaf-1, true, Monitor::_safepoint_check_never); - def(SharedDictionary_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // lookups done by VM thread + def(SharedDictionary_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); def(Module_lock , PaddedMutex , leaf+2, true, Monitor::_safepoint_check_always); def(InlineCacheBuffer_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); def(VMStatistic_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); - def(ExpandHeap_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // Used during compilation by VM thread + def(ExpandHeap_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // Used during compilation by VM thread def(JNIHandleBlockFreeList_lock , PaddedMutex , leaf-1, true, Monitor::_safepoint_check_never); // handles are used by VM thread def(SignatureHandlerLibrary_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); def(SymbolArena_lock , PaddedMutex , leaf+2, true, Monitor::_safepoint_check_never); - def(ProfilePrint_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // serial profile printing - def(ExceptionCache_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // serial profile printing + def(ProfilePrint_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // serial profile printing + def(ExceptionCache_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // serial profile printing def(OsrList_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); def(Debug1_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); #ifndef PRODUCT - def(FullGCALot_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // a lock to make FullGCALot MT safe + def(FullGCALot_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); // a lock to make FullGCALot MT safe #endif def(BeforeExit_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_always); - def(PerfDataMemAlloc_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for allocating PerfData memory for performance data - def(PerfDataManager_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for synchronized access to PerfDataManager resources + def(PerfDataMemAlloc_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for allocating PerfData memory for performance data + def(PerfDataManager_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for synchronized access to PerfDataManager resources // CMS_modUnionTable_lock leaf // CMS_bitMap_lock leaf 1 // CMS_freeList_lock leaf 2 - def(Threads_lock , PaddedMonitor, barrier, true, Monitor::_safepoint_check_sometimes); + def(Threads_lock , PaddedMonitor, barrier, true, Monitor::_safepoint_check_sometimes); // Used for safepoint protocol. def(NonJavaThreadsList_lock , PaddedMutex, leaf, true, Monitor::_safepoint_check_never); def(NonJavaThreadsListSync_lock , PaddedMutex, leaf, true, Monitor::_safepoint_check_never); - def(VMOperationQueue_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_sometimes); // VM_thread allowed to block on these - def(VMOperationRequest_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_sometimes); + def(VMOperationQueue_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_never); // VM_thread allowed to block on these + def(VMOperationRequest_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_always); def(RetData_lock , PaddedMutex , nonleaf, false, Monitor::_safepoint_check_always); - def(Terminator_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_sometimes); + def(Terminator_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_always); def(InitCompleted_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_never); def(VtableStubs_lock , PaddedMutex , nonleaf, true, Monitor::_safepoint_check_never); def(Notify_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_always); @@ -295,11 +295,11 @@ def(JNIGlobalActive_lock , PaddedMutex , nonleaf-1, true, Monitor::_safepoint_check_never); def(JNIWeakAlloc_lock , PaddedMutex , nonleaf, true, Monitor::_safepoint_check_never); def(JNIWeakActive_lock , PaddedMutex , nonleaf-1, true, Monitor::_safepoint_check_never); - def(JNICritical_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_always); // used for JNI critical regions + def(JNICritical_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_always); // used for JNI critical regions def(AdapterHandlerLibrary_lock , PaddedMutex , nonleaf, true, Monitor::_safepoint_check_always); - def(Heap_lock , PaddedMonitor, nonleaf+1, false, Monitor::_safepoint_check_sometimes); - def(JfieldIdCreation_lock , PaddedMutex , nonleaf+1, true, Monitor::_safepoint_check_always); // jfieldID, Used in VM_Operation + def(Heap_lock , PaddedMonitor, nonleaf+1, false, Monitor::_safepoint_check_sometimes); // Doesn't safepoint check during termination. + def(JfieldIdCreation_lock , PaddedMutex , nonleaf+1, true, Monitor::_safepoint_check_always); // jfieldID, Used in VM_Operation def(CompiledIC_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_never); // locks VtableStubs_lock, InlineCacheBuffer_lock def(CompileTaskAlloc_lock , PaddedMutex , nonleaf+2, true, Monitor::_safepoint_check_always); @@ -307,8 +307,8 @@ def(DirectivesStack_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); def(MultiArray_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_always); - def(JvmtiThreadState_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_always); // Used by JvmtiThreadState/JvmtiEventController - def(Management_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_always); // used for JVM management + def(JvmtiThreadState_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_always); // Used by JvmtiThreadState/JvmtiEventController + def(Management_lock , PaddedMutex , nonleaf+2, false, Monitor::_safepoint_check_always); // used for JVM management def(Compile_lock , PaddedMutex , nonleaf+3, true, Monitor::_safepoint_check_always); def(MethodData_lock , PaddedMutex , nonleaf+3, false, Monitor::_safepoint_check_always); @@ -318,7 +318,7 @@ def(Debug2_lock , PaddedMutex , nonleaf+4, true, Monitor::_safepoint_check_never); def(Debug3_lock , PaddedMutex , nonleaf+4, true, Monitor::_safepoint_check_never); def(CompileThread_lock , PaddedMonitor, nonleaf+5, false, Monitor::_safepoint_check_always); - def(PeriodicTask_lock , PaddedMonitor, nonleaf+5, true, Monitor::_safepoint_check_sometimes); + def(PeriodicTask_lock , PaddedMonitor, nonleaf+5, true, Monitor::_safepoint_check_always); def(RedefineClasses_lock , PaddedMonitor, nonleaf+5, true, Monitor::_safepoint_check_always); if (WhiteBoxAPI) { @@ -329,7 +329,7 @@ def(JfrMsg_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_always); def(JfrBuffer_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); def(JfrStream_lock , PaddedMutex , leaf+1, true, Monitor::_safepoint_check_never); // ensure to rank lower than 'safepoint' - def(JfrStacktrace_lock , PaddedMutex , special, true, Monitor::_safepoint_check_sometimes); + def(JfrStacktrace_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); def(JfrThreadSampler_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_never); #endif @@ -338,8 +338,7 @@ #endif def(CodeHeapStateAnalytics_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); - - def(NMethodSweeperStats_lock , PaddedMutex , special, true, Monitor::_safepoint_check_sometimes); + def(NMethodSweeperStats_lock , PaddedMutex , special, true, Monitor::_safepoint_check_never); def(ThreadsSMRDelete_lock , PaddedMonitor, special, false, Monitor::_safepoint_check_never); def(SharedDecoder_lock , PaddedMutex , native, false, Monitor::_safepoint_check_never); def(DCmdFactory_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); diff -r afce4a27f2c2 -r f03d5a093093 src/hotspot/share/runtime/vmThread.cpp --- a/src/hotspot/share/runtime/vmThread.cpp Wed May 01 07:12:14 2019 -0400 +++ b/src/hotspot/share/runtime/vmThread.cpp Wed May 01 08:56:38 2019 -0400 @@ -623,10 +623,8 @@ } // - // Notify (potential) waiting Java thread(s) - lock without safepoint - // check so that sneaking is not possible - { MutexLocker mu(VMOperationRequest_lock, - Mutex::_no_safepoint_check_flag); + // Notify (potential) waiting Java thread(s) + { MutexLocker mu(VMOperationRequest_lock, Mutex::_no_safepoint_check_flag); VMOperationRequest_lock->notify_all(); } diff -r afce4a27f2c2 -r f03d5a093093 test/hotspot/jtreg/runtime/Shutdown/ShutdownTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/hotspot/jtreg/runtime/Shutdown/ShutdownTest.java Wed May 01 08:56:38 2019 -0400 @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 4656449 4945125 8074355 + * @summary Make MutexLocker smarter about non-Java threads + * @library /test/lib + * @run driver/timeout=240 ShutdownTest + */ + +// This test is adapted from an old regression test for bug 4945125, where VerifyBeforeExit +// crashes before exit for the regression test for bug 4656449. +// The fix is to acquire the Heap_lock before exit after the JavaThread is removed from +// the threads list. This fix is still valid. This code requires Heap_lock be acquired +// without a safepoint check at exit. + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +public class ShutdownTest { + Object[] obj; + + ShutdownTest() { + // Allocate to get some GC pressure. + obj = new Object[100000]; + } + + static class ShutdownTestThread extends Thread { + public void run() { + while (true) { + ShutdownTest st = new ShutdownTest(); + } + } + + public static void main(String args[]) { + System.out.println("- ShutdownTest -"); + + for (int i = 0; i < 100; i++) { + ShutdownTestThread st = new ShutdownTestThread(); + st.setDaemon(true); + st.start(); + } + } + } + + private static void startVM(List options) throws Throwable { + // Combine VM flags given from command-line and your additional options + OutputAnalyzer output = ProcessTools.executeTestJvm(options.toArray(new String[options.size()])); + output.shouldContain("- ShutdownTest -"); + output.shouldHaveExitValue(0); + + } + + public static void main(String[] args) throws Throwable { + List options = new ArrayList<>(); + + Collections.addAll(options, + "-Xmx2500k", + "-XX:+UnlockDiagnosticVMOptions", + "-XX:+VerifyBeforeExit"); + options.add(ShutdownTestThread.class.getName()); + + for (int iteration = 0; iteration < 5; ++iteration) { + startVM(options); + } + } +}