8184732: Deadlock detection improvements for 'special' locks
Summary: Assert that special ranked locks cannot safepoint and allow_vm_block and remove locks from the exceptional lock list in no_safepoint_verifier.
Reviewed-by: dholmes, eosterlund
--- a/src/hotspot/share/oops/instanceKlass.cpp Mon Sep 30 18:02:24 2019 +0200
+++ b/src/hotspot/share/oops/instanceKlass.cpp Mon Sep 30 13:10:11 2019 -0400
@@ -1101,7 +1101,7 @@
void InstanceKlass::set_implementor(Klass* k) {
- assert_lock_strong(Compile_lock);
+ assert_locked_or_safepoint(Compile_lock);
assert(is_interface(), "not interface");
Klass* volatile* addr = adr_implementor();
assert(addr != NULL, "null addr");
@@ -2333,8 +2333,8 @@
// being added to class hierarchy (see SystemDictionary:::add_to_hierarchy()).
_init_state = allocated;
- {
- MutexLocker ml(Compile_lock);
+ { // Otherwise this needs to take out the Compile_lock.
+ assert(SafepointSynchronize::is_at_safepoint(), "only called at safepoint");
init_implementor();
}
--- a/src/hotspot/share/prims/whitebox.cpp Mon Sep 30 18:02:24 2019 +0200
+++ b/src/hotspot/share/prims/whitebox.cpp Mon Sep 30 13:10:11 2019 -0400
@@ -1745,6 +1745,20 @@
sfpt_check_attempted);
WB_END
+WB_ENTRY(void, WB_AssertSpecialLock(JNIEnv* env, jobject o, jboolean allowVMBlock, jboolean safepointCheck))
+ // Create a special lock violating condition in value
+ Mutex::SafepointCheckRequired sfpt_check_required = safepointCheck ?
+ Mutex::_safepoint_check_always :
+ Mutex::_safepoint_check_never;
+ Mutex::SafepointCheckFlag safepoint_check = safepointCheck ?
+ Monitor::_safepoint_check_flag :
+ Monitor::_no_safepoint_check_flag;
+
+ MutexLocker ml(new Mutex(Mutex::special, "SpecialTest_lock", allowVMBlock, sfpt_check_required), safepoint_check);
+ // If the lock above succeeds, try to safepoint to test the NSV implied with this special lock.
+ ThreadBlockInVM tbivm(JavaThread::current());
+WB_END
+
WB_ENTRY(jboolean, WB_IsMonitorInflated(JNIEnv* env, jobject wb, jobject obj))
oop obj_oop = JNIHandles::resolve(obj);
return (jboolean) obj_oop->mark().has_monitor();
@@ -2322,6 +2336,7 @@
{CC"AddModuleExportsToAll", CC"(Ljava/lang/Object;Ljava/lang/String;)V",
(void*)&WB_AddModuleExportsToAll },
{CC"assertMatchingSafepointCalls", CC"(ZZ)V", (void*)&WB_AssertMatchingSafepointCalls },
+ {CC"assertSpecialLock", CC"(ZZ)V", (void*)&WB_AssertSpecialLock },
{CC"isMonitorInflated0", CC"(Ljava/lang/Object;)Z", (void*)&WB_IsMonitorInflated },
{CC"forceSafepoint", CC"()V", (void*)&WB_ForceSafepoint },
{CC"getConstantPool0", CC"(Ljava/lang/Class;)J", (void*)&WB_GetConstantPool },
--- a/src/hotspot/share/runtime/mutex.cpp Mon Sep 30 18:02:24 2019 +0200
+++ b/src/hotspot/share/runtime/mutex.cpp Mon Sep 30 13:10:11 2019 -0400
@@ -282,6 +282,11 @@
assert(_safepoint_check_required != _safepoint_check_sometimes || is_sometimes_ok(name),
"Lock has _safepoint_check_sometimes %s", name);
+
+ assert(_rank > special || _allow_vm_block,
+ "Special locks or below should allow the vm to block");
+ assert(_rank > special || _safepoint_check_required == _safepoint_check_never,
+ "Special locks or below should never safepoint");
#endif
}
@@ -388,17 +393,13 @@
// NSV implied with locking allow_vm_block or !safepoint_check locks.
void Mutex::no_safepoint_verifier(Thread* thread, bool enable) {
- // Threads_lock is special, since the safepoint synchronization will not start before this is
- // acquired. Hence, a JavaThread cannot be holding it at a safepoint. So is VMOperationRequest_lock,
- // since it is used to transfer control between JavaThreads and the VMThread
- // Do not *exclude* any locks unless you are absolutely sure it is correct. Ask someone else first!
- if ((_allow_vm_block &&
- this != Threads_lock &&
- this != Compile_lock && // Temporary: should not be necessary when we get separate compilation
- this != tty_lock && // The tty_lock is released for the safepoint.
- this != VMOperationRequest_lock &&
- this != VMOperationQueue_lock) ||
- rank() == Mutex::special) {
+ // The tty_lock is special because it is released for the safepoint by
+ // the safepoint mechanism.
+ if (this == tty_lock) {
+ return;
+ }
+
+ if (_allow_vm_block) {
if (enable) {
thread->_no_safepoint_count++;
} else {
--- a/src/hotspot/share/runtime/mutexLocker.cpp Mon Sep 30 18:02:24 2019 +0200
+++ b/src/hotspot/share/runtime/mutexLocker.cpp Mon Sep 30 13:10:11 2019 -0400
@@ -288,7 +288,7 @@
def(JvmtiThreadState_lock , PaddedMutex , nonleaf+2, false, _safepoint_check_always); // Used by JvmtiThreadState/JvmtiEventController
def(Management_lock , PaddedMutex , nonleaf+2, false, _safepoint_check_always); // used for JVM management
- def(Compile_lock , PaddedMutex , nonleaf+3, true, _safepoint_check_always);
+ def(Compile_lock , PaddedMutex , nonleaf+3, false, _safepoint_check_always);
def(MethodData_lock , PaddedMutex , nonleaf+3, false, _safepoint_check_always);
def(TouchedMethodLog_lock , PaddedMutex , nonleaf+3, false, _safepoint_check_always);
--- a/src/hotspot/share/runtime/vmThread.cpp Mon Sep 30 18:02:24 2019 +0200
+++ b/src/hotspot/share/runtime/vmThread.cpp Mon Sep 30 13:10:11 2019 -0400
@@ -354,9 +354,9 @@
void VMThread::wait_for_vm_thread_exit() {
assert(Thread::current()->is_Java_thread(), "Should be a JavaThread");
assert(((JavaThread*)Thread::current())->is_terminated(), "Should be terminated");
- { MutexLocker mu(VMOperationQueue_lock, Mutex::_no_safepoint_check_flag);
+ { MonitorLocker mu(VMOperationQueue_lock, Mutex::_no_safepoint_check_flag);
_should_terminate = true;
- VMOperationQueue_lock->notify();
+ mu.notify();
}
// Note: VM thread leaves at Safepoint. We are not stopped by Safepoint
@@ -620,8 +620,8 @@
//
// Notify (potential) waiting Java thread(s)
- { MutexLocker mu(VMOperationRequest_lock, Mutex::_no_safepoint_check_flag);
- VMOperationRequest_lock->notify_all();
+ { MonitorLocker mu(VMOperationRequest_lock, Mutex::_no_safepoint_check_flag);
+ mu.notify_all();
}
// We want to make sure that we get to a safepoint regularly
@@ -695,12 +695,11 @@
// VMOperationQueue_lock, so we can block without a safepoint check. This allows vm operation requests
// to be queued up during a safepoint synchronization.
{
- VMOperationQueue_lock->lock_without_safepoint_check();
+ MonitorLocker ml(VMOperationQueue_lock, Mutex::_no_safepoint_check_flag);
log_debug(vmthread)("Adding VM operation: %s", op->name());
_vm_queue->add(op);
op->set_timestamp(os::javaTimeMillis());
- VMOperationQueue_lock->notify();
- VMOperationQueue_lock->unlock();
+ ml.notify();
}
if (!concurrent) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/Safepoint/NoSafepointVerifier.java Mon Sep 30 13:10:11 2019 -0400
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2018, 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 8184732
+ * @summary Ensure that special locks never safepoint check and are vm_block.
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * java.management
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main NoSafepointVerifier
+ */
+
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.Platform;
+
+import sun.hotspot.WhiteBox;
+
+public class NoSafepointVerifier {
+
+ static void runTest(String test) throws Exception {
+ if (Platform.isDebugBuild()){
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+ "-Xbootclasspath/a:.",
+ "-XX:+UnlockDiagnosticVMOptions",
+ "-XX:+WhiteBoxAPI",
+ "-XX:-CreateCoredumpOnCrash",
+ "NoSafepointVerifier",
+ test);
+ OutputAnalyzer output = new OutputAnalyzer(pb.start());
+ output.shouldContain(test);
+ }
+ }
+
+ static String test1 = "Special locks or below should never safepoint";
+ static String test2 = "Special locks or below should allow the vm to block";
+ static String test3 = "Possible safepoint reached by thread that does not allow it";
+
+ public static void main(String args[]) throws Exception {
+ if (args.length > 0) {
+ if (args[0].equals(test1)) {
+ WhiteBox.getWhiteBox().assertSpecialLock(/*vm_block*/true, /*safepoint_check_always*/true);
+ } else if (args[0].equals(test2)) {
+ WhiteBox.getWhiteBox().assertSpecialLock(/*vm_block*/false, /*safepoint_check_always*/false);
+ } else if (args[0].equals(test3)) {
+ WhiteBox.getWhiteBox().assertSpecialLock(/*vm_block*/true, /*safepoint_check_always*/false);
+ }
+ } else {
+ runTest(test1);
+ runTest(test2);
+ runTest(test3);
+ }
+ }
+}
--- a/test/lib/sun/hotspot/WhiteBox.java Mon Sep 30 18:02:24 2019 +0200
+++ b/test/lib/sun/hotspot/WhiteBox.java Mon Sep 30 13:10:11 2019 -0400
@@ -511,6 +511,7 @@
// Safepoint Checking
public native void assertMatchingSafepointCalls(boolean mutexSafepointValue, boolean attemptedNoSafepointValue);
+ public native void assertSpecialLock(boolean allowVMBlock, boolean safepointCheck);
// Sharing & archiving
public native String getDefaultArchivePath();