Revert^18 "Thread suspension cleanup and deadlock fix"

This reverts commit 8bc6a58df7046b4d6f4b51eb274c7e60fea396ff.

PS1 is identical to
  https://android-review.git.corp.google.com/c/platform/art/+/2746640

PS2 makes the following changes:
- Remove one DCHECK each from the two WaitForFlipFunction variants.
  The DCHECK could fail if another GC was started in the interim.
- Break up the WaitForSuspendBarrier timeout into shorter ones.
  so we don't time out as easily if our process is frozen.
- Include the thread name for ThreadSuspendByThreadIdWarning, since
  we don't get complete tombstones for some failures.

Test: Treehugger, host tests.
Bug: 240742796
Bug: 203363895
Bug: 238032384
Bug: 253671779
Bug: 276660630
Bug: 295880862
Bug: 294334417
Bug: 301090887
Bug: 313347640
(and more)

Change-Id: I12c5c01b1e006baab4ee4148aadbc721723fb89e

1 files changed, 7 insertions, 5 deletions

diff --git a/openjdkjvmti/ti_stack.cc b/openjdkjvmti/ti_stack.cc
index 9af8861260..a56081408a 100644
--- a/openjdkjvmti/ti_stack.cc
+++ b/openjdkjvmti/ti_stack.cc
@@ -363,7 +363,13 @@ static void RunCheckpointAndWait(Data* data, size_t max_frame_count)
     REQUIRES_SHARED(art::Locks::mutator_lock_) {
   // Note: requires the mutator lock as the checkpoint requires the mutator lock.
   GetAllStackTracesVectorClosure<Data> closure(max_frame_count, data);
-  size_t barrier_count = art::Runtime::Current()->GetThreadList()->RunCheckpoint(&closure, nullptr);
+  // TODO(b/253671779): Replace this use of RunCheckpointUnchecked() with RunCheckpoint(). This is
+  // currently not possible, since the following undesirable call chain (abbreviated here) is then
+  // possible and exercised by current tests: (jvmti) GetAllStackTraces -> <this function> ->
+  // RunCheckpoint -> GetStackTraceVisitor -> EncodeMethodId -> Class::EnsureMethodIds ->
+  // Class::Alloc -> AllocObjectWithAllocator -> potentially suspends, or runs GC, etc. -> CHECK
+  // failure.
+  size_t barrier_count = art::Runtime::Current()->GetThreadList()->RunCheckpointUnchecked(&closure);
   if (barrier_count == 0) {
     return;
   }
@@ -544,7 +550,6 @@ jvmtiError StackUtil::GetThreadListStackTraces(jvmtiEnv* env,
           // Found the thread.
           art::MutexLock mu(self, mutex);
 
-          threads.push_back(thread);
           thread_list_indices.push_back(index);
 
           frames.emplace_back(new std::vector<jvmtiFrameInfo>());
@@ -562,7 +567,6 @@ jvmtiError StackUtil::GetThreadListStackTraces(jvmtiEnv* env,
 
     // Storage. Only access directly after completion.
 
-    std::vector<art::Thread*> threads;
     std::vector<size_t> thread_list_indices;
 
     std::vector<std::unique_ptr<std::vector<jvmtiFrameInfo>>> frames;
@@ -600,12 +604,10 @@ jvmtiError StackUtil::GetThreadListStackTraces(jvmtiEnv* env,
     jvmtiStackInfo& stack_info = stack_info_array.get()[index];
     memset(&stack_info, 0, sizeof(jvmtiStackInfo));
 
-    art::Thread* self = data.threads[index];
     const std::vector<jvmtiFrameInfo>& thread_frames = *data.frames[index].get();
 
     // For the time being, set the thread to null. We don't have good ScopedLocalRef
     // infrastructure.
-    DCHECK(self->GetPeerFromOtherThread() != nullptr);
     stack_info.thread = nullptr;
     stack_info.state = JVMTI_THREAD_STATE_SUSPENDED;