Force stack dump to diagnose empty checkpoint timeout (2).

Bug: 33006388
Bug: 12687968
Test: test-art-host
Test: Thread dumping in a simulated empty checkpoint timeout.

Change-Id: I06641396b8f3d7a1a98366a01807aab2e6f31bd5
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 6e0569b..c737fe4 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -96,13 +96,17 @@
   return false;
 }
 
-StackVisitor::StackVisitor(Thread* thread, Context* context, StackWalkKind walk_kind)
-    : StackVisitor(thread, context, walk_kind, 0) {}
+StackVisitor::StackVisitor(Thread* thread,
+                           Context* context,
+                           StackWalkKind walk_kind,
+                           bool check_suspended)
+    : StackVisitor(thread, context, walk_kind, 0, check_suspended) {}
 
 StackVisitor::StackVisitor(Thread* thread,
                            Context* context,
                            StackWalkKind walk_kind,
-                           size_t num_frames)
+                           size_t num_frames,
+                           bool check_suspended)
     : thread_(thread),
       walk_kind_(walk_kind),
       cur_shadow_frame_(nullptr),
@@ -112,8 +116,11 @@
       num_frames_(num_frames),
       cur_depth_(0),
       current_inlining_depth_(0),
-      context_(context) {
-  DCHECK(thread == Thread::Current() || thread->IsSuspended()) << *thread;
+      context_(context),
+      check_suspended_(check_suspended) {
+  if (check_suspended_) {
+    DCHECK(thread == Thread::Current() || thread->IsSuspended()) << *thread;
+  }
 }
 
 InlineInfo StackVisitor::GetCurrentInlineInfo() const {
@@ -788,7 +795,9 @@
 
 template <StackVisitor::CountTransitions kCount>
 void StackVisitor::WalkStack(bool include_transitions) {
-  DCHECK(thread_ == Thread::Current() || thread_->IsSuspended());
+  if (check_suspended_) {
+    DCHECK(thread_ == Thread::Current() || thread_->IsSuspended());
+  }
   CHECK_EQ(cur_depth_, 0U);
   bool exit_stubs_installed = Runtime::Current()->GetInstrumentation()->AreExitStubsInstalled();
   uint32_t instrumentation_stack_depth = 0;
diff --git a/runtime/stack.h b/runtime/stack.h
index 9dceb29..90a0aee 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -590,7 +590,10 @@
   };
 
  protected:
-  StackVisitor(Thread* thread, Context* context, StackWalkKind walk_kind);
+  StackVisitor(Thread* thread,
+               Context* context,
+               StackWalkKind walk_kind,
+               bool check_suspended = true);
 
   bool GetRegisterIfAccessible(uint32_t reg, VRegKind kind, uint32_t* val) const
       REQUIRES_SHARED(Locks::mutator_lock_);
@@ -797,7 +800,11 @@
 
  private:
   // Private constructor known in the case that num_frames_ has already been computed.
-  StackVisitor(Thread* thread, Context* context, StackWalkKind walk_kind, size_t num_frames)
+  StackVisitor(Thread* thread,
+               Context* context,
+               StackWalkKind walk_kind,
+               size_t num_frames,
+               bool check_suspended = true)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   bool IsAccessibleRegister(uint32_t reg, bool is_float) const {
@@ -851,6 +858,7 @@
 
  protected:
   Context* const context_;
+  const bool check_suspended_;
 };
 
 }  // namespace art
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 632a380..7e84b55 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1583,15 +1583,24 @@
 }
 
 struct StackDumpVisitor : public StackVisitor {
-  StackDumpVisitor(std::ostream& os_in, Thread* thread_in, Context* context, bool can_allocate_in)
+  StackDumpVisitor(std::ostream& os_in,
+                   Thread* thread_in,
+                   Context* context,
+                   bool can_allocate_in,
+                   bool check_suspended = true,
+                   bool dump_locks_in = true)
       REQUIRES_SHARED(Locks::mutator_lock_)
-      : StackVisitor(thread_in, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
+      : StackVisitor(thread_in,
+                     context,
+                     StackVisitor::StackWalkKind::kIncludeInlinedFrames,
+                     check_suspended),
         os(os_in),
         can_allocate(can_allocate_in),
         last_method(nullptr),
         last_line_number(0),
         repetition_count(0),
-        frame_count(0) {}
+        frame_count(0),
+        dump_locks(dump_locks_in) {}
 
   virtual ~StackDumpVisitor() {
     if (frame_count == 0) {
@@ -1636,8 +1645,10 @@
       if (frame_count == 0) {
         Monitor::DescribeWait(os, GetThread());
       }
-      if (can_allocate) {
+      if (can_allocate && dump_locks) {
         // Visit locks, but do not abort on errors. This would trigger a nested abort.
+        // Skip visiting locks if dump_locks is false as it would cause a bad_mutexes_held in
+        // RegTypeCache::RegTypeCache due to thread_list_lock.
         Monitor::VisitLocks(this, DumpLockedObject, &os, false);
       }
     }
@@ -1681,6 +1692,7 @@
   int last_line_number;
   int repetition_count;
   int frame_count;
+  const bool dump_locks;
 };
 
 static bool ShouldShowNativeStack(const Thread* thread)
@@ -1712,7 +1724,7 @@
   return current_method != nullptr && current_method->IsNative();
 }
 
-void Thread::DumpJavaStack(std::ostream& os) const {
+void Thread::DumpJavaStack(std::ostream& os, bool check_suspended, bool dump_locks) const {
   // If flip_function is not null, it means we have run a checkpoint
   // before the thread wakes up to execute the flip function and the
   // thread roots haven't been forwarded.  So the following access to
@@ -1741,7 +1753,7 @@
 
   std::unique_ptr<Context> context(Context::Create());
   StackDumpVisitor dumper(os, const_cast<Thread*>(this), context.get(),
-                          !tls32_.throwing_OutOfMemoryError);
+                          !tls32_.throwing_OutOfMemoryError, check_suspended, dump_locks);
   dumper.WalkStack();
 
   if (have_exception) {
@@ -1767,10 +1779,15 @@
     // If we're currently in native code, dump that stack before dumping the managed stack.
     if (dump_native_stack && (dump_for_abort || force_dump_stack || ShouldShowNativeStack(this))) {
       DumpKernelStack(os, GetTid(), "  kernel: ", false);
-      ArtMethod* method = GetCurrentMethod(nullptr, !(dump_for_abort || force_dump_stack));
+      ArtMethod* method =
+          GetCurrentMethod(nullptr,
+                           /*check_suspended*/ !force_dump_stack,
+                           /*abort_on_error*/ !(dump_for_abort || force_dump_stack));
       DumpNativeStack(os, GetTid(), backtrace_map, "  native: ", method);
     }
-    DumpJavaStack(os);
+    DumpJavaStack(os,
+                  /*check_suspended*/ !force_dump_stack,
+                  /*dump_locks*/ !force_dump_stack);
   } else {
     os << "Not able to dump stack of thread that isn't suspended";
   }
@@ -2918,9 +2935,12 @@
 // Note: this visitor may return with a method set, but dex_pc_ being DexFile:kDexNoIndex. This is
 //       so we don't abort in a special situation (thinlocked monitor) when dumping the Java stack.
 struct CurrentMethodVisitor FINAL : public StackVisitor {
-  CurrentMethodVisitor(Thread* thread, Context* context, bool abort_on_error)
+  CurrentMethodVisitor(Thread* thread, Context* context, bool check_suspended, bool abort_on_error)
       REQUIRES_SHARED(Locks::mutator_lock_)
-      : StackVisitor(thread, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
+      : StackVisitor(thread,
+                     context,
+                     StackVisitor::StackWalkKind::kIncludeInlinedFrames,
+                     check_suspended),
         this_object_(nullptr),
         method_(nullptr),
         dex_pc_(0),
@@ -2944,8 +2964,13 @@
   const bool abort_on_error_;
 };
 
-ArtMethod* Thread::GetCurrentMethod(uint32_t* dex_pc, bool abort_on_error) const {
-  CurrentMethodVisitor visitor(const_cast<Thread*>(this), nullptr, abort_on_error);
+ArtMethod* Thread::GetCurrentMethod(uint32_t* dex_pc,
+                                    bool check_suspended,
+                                    bool abort_on_error) const {
+  CurrentMethodVisitor visitor(const_cast<Thread*>(this),
+                               nullptr,
+                               check_suspended,
+                               abort_on_error);
   visitor.WalkStack(false);
   if (dex_pc != nullptr) {
     *dex_pc = visitor.dex_pc_;
diff --git a/runtime/thread.h b/runtime/thread.h
index b59eac6..40aba04 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -201,7 +201,9 @@
       REQUIRES(!Locks::thread_suspend_count_lock_)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
-  void DumpJavaStack(std::ostream& os) const
+  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_);
 
@@ -411,7 +413,9 @@
 
   // Get the current method and dex pc. If there are errors in retrieving the dex pc, this will
   // abort the runtime iff abort_on_error is true.
-  ArtMethod* GetCurrentMethod(uint32_t* dex_pc, bool abort_on_error = true) const
+  ArtMethod* GetCurrentMethod(uint32_t* dex_pc,
+                              bool check_suspended = true,
+                              bool abort_on_error = true) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Returns whether the given exception was thrown by the current Java method being executed