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
Merged-In: Ib55eafba72c5d15de01719840ba3f223ae4af8c7
Change-Id: Ib55eafba72c5d15de01719840ba3f223ae4af8c7

3 files changed, 29 insertions, 11 deletions

diff --git a/runtime/runtime_test.cc b/runtime/runtime_test.cc
index aa4020e25f..282e43071e 100644
--- a/runtime/runtime_test.cc
+++ b/runtime/runtime_test.cc
@@ -40,4 +40,18 @@ TEST_F(RuntimeTest, AbortWithThreadListLockHeld) {
   }, 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 03fbd20235..70ed7c8038 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1861,13 +1861,21 @@ void Thread::DumpState(std::ostream& os, const Thread* thread, pid_t tid) {
   }
 
   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 7c8ad24252..dd483c188d 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -221,19 +221,16 @@ class Thread {
             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 @@ class Thread {
                  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.