ART: Allow thread suspend lock to be held when dumping a thread
Only dumping a thread's suspend state requires holding the lock.
Specialize the code to recognize if the lock is already held,
and remove the negative capability from the callers.
Bug: 134037466
Bug: 134167395
Test: m
Test: m test-art-host-gtest-runtime_test
Change-Id: Ib55eafba72c5d15de01719840ba3f223ae4af8c7
diff --git a/runtime/runtime_test.cc b/runtime/runtime_test.cc
index aa4020e..282e430 100644
--- a/runtime/runtime_test.cc
+++ b/runtime/runtime_test.cc
@@ -40,4 +40,18 @@
}, kDeathRegex);
}
+
+TEST_F(RuntimeTest, AbortWithThreadSuspendCountLockHeld) {
+ // This assumes the test is run single-threaded: do not start the runtime to avoid daemon threads.
+
+ constexpr const char* kDeathRegex = "Skipping all-threads dump as locks are held";
+ ASSERT_DEATH({
+ // The regex only works if we can ensure output goes to stderr.
+ android::base::SetLogger(android::base::StderrLogger);
+
+ MutexLock mu(Thread::Current(), *Locks::thread_suspend_count_lock_);
+ Runtime::Abort("Attempt to abort");
+ }, kDeathRegex);
+}
+
} // namespace art
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 6c1f71c..fbbcc90 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1863,13 +1863,21 @@
}
if (thread != nullptr) {
- MutexLock mu(self, *Locks::thread_suspend_count_lock_);
- os << " | group=\"" << group_name << "\""
- << " sCount=" << thread->tls32_.suspend_count
- << " dsCount=" << thread->tls32_.debug_suspend_count
- << " flags=" << thread->tls32_.state_and_flags.as_struct.flags
- << " obj=" << reinterpret_cast<void*>(thread->tlsPtr_.opeer)
- << " self=" << reinterpret_cast<const void*>(thread) << "\n";
+ auto suspend_log_fn = [&]() REQUIRES(Locks::thread_suspend_count_lock_) {
+ os << " | group=\"" << group_name << "\""
+ << " sCount=" << thread->tls32_.suspend_count
+ << " dsCount=" << thread->tls32_.debug_suspend_count
+ << " flags=" << thread->tls32_.state_and_flags.as_struct.flags
+ << " obj=" << reinterpret_cast<void*>(thread->tlsPtr_.opeer)
+ << " self=" << reinterpret_cast<const void*>(thread) << "\n";
+ };
+ if (Locks::thread_suspend_count_lock_->IsExclusiveHeld(self)) {
+ Locks::thread_suspend_count_lock_->AssertExclusiveHeld(self); // For annotalysis.
+ suspend_log_fn();
+ } else {
+ MutexLock mu(self, *Locks::thread_suspend_count_lock_);
+ suspend_log_fn();
+ }
}
os << " | sysTid=" << tid
diff --git a/runtime/thread.h b/runtime/thread.h
index 4991566..c4a9bcf 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -221,19 +221,16 @@
bool dump_native_stack = true,
BacktraceMap* backtrace_map = nullptr,
bool force_dump_stack = false) const
- REQUIRES(!Locks::thread_suspend_count_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
void DumpJavaStack(std::ostream& os,
bool check_suspended = true,
bool dump_locks = true) const
- REQUIRES(!Locks::thread_suspend_count_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
// Dumps the SIGQUIT per-thread header. 'thread' can be null for a non-attached thread, in which
// case we use 'tid' to identify the thread, and we'll include as much information as we can.
static void DumpState(std::ostream& os, const Thread* thread, pid_t tid)
- REQUIRES(!Locks::thread_suspend_count_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
ThreadState GetState() const {
@@ -1342,7 +1339,6 @@
bool dump_native_stack = true,
BacktraceMap* backtrace_map = nullptr,
bool force_dump_stack = false) const
- REQUIRES(!Locks::thread_suspend_count_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
// Out-of-line conveniences for debugging in gdb.