Revert^10 "Thread suspension cleanup and deadlock fix"""

This reverts commit 63af30b8fe8d4e1dc32db4dcb5e5dae1efdc7f31.

master (aosp/2530206) PS1 is identical to aosp/2377951 .

master (aosp/2530206) PS2 is a rebase.

At this point, master branch was replaced by main, and this CL moved.

PS1: Restructure documentation for the IncrementSuspendCount handshake
     to install a suspend barrier.
     Document a couple of additional mutator lock assumptions.
     Add some DCHECKs to check that suspended threads really are
     suspended.
     Weaken seq_cst memory order in a couple of places where it really
     didn't make sense here. Clearly not a correctness fix.
     Includes a rebase and merge with aosp/2587606.
PS2: Another rebase.
     Fix thumb assembler test to compensate for Thread structure layout
     changes.
PS3: Messy rebase, primarily to handle aosp/2670108, which included both
     new fixes around this and a few small snippets of this CL.
     Call EnsureFlipFunctionStarted without a state-and-flags argument
     only when we actually hold the mutator lock, as promised.
PS4: Minor rebase, some lint fixes.
PS5: Another minor lint fix that I had missed in PS4.
PS6: Fix for RunCheckpoint bug introduced around PS3. Fix expectations
     in jni_cfi_test to compensate for thread structure layout changes.
PS7: In PS3+, EnsureFlipFunctionStarted could access a destroyed "this"
     thread. Fix that, and make the function static to make this
     constraint more explicit. (And running a method on a potentially
     destroyed object just seemed unclean.)
PS8: Address reviewer comments. The major issue was that we released
     the suspend_count_lock too early in FlipThreadRoots, potentially
     allowing an intervening SuspendAll to block us. The fix involved
     a very minor extention of the mutex API.
PS9: Comment typo fix.
PS10: Address new reviewer comments. Rebase.

MUST_SLEEP for 129_ThreadGetId debug output.
Test: Treehugger.
Bug: 240742796
Bug: 203363895
Bug: 238032384
Bug: 253671779
Bug: 276660630
(and more)

Change-Id: I0f2450e394c03c17eece3698286b2f3e45727967

1 files changed, 39 insertions, 36 deletions

diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h
index dba1e1299b..712b61d4ac 100644
--- a/runtime/base/mutex-inl.h
+++ b/runtime/base/mutex-inl.h
@@ -60,43 +60,44 @@ static inline void CheckUnattachedThread(LockLevel level) NO_THREAD_SAFETY_ANALY
   // The check below enumerates the cases where we expect not to be able to check the validity of
   // locks on a thread. Lock checking is disabled to avoid deadlock when checking shutdown lock.
   // TODO: tighten this check.
-  if (kDebugLocking) {
-    CHECK(!Locks::IsSafeToCallAbortRacy() ||
-          // Used during thread creation to avoid races with runtime shutdown. Thread::Current not
-          // yet established.
-          level == kRuntimeShutdownLock ||
-          // Thread Ids are allocated/released before threads are established.
-          level == kAllocatedThreadIdsLock ||
-          // Thread LDT's are initialized without Thread::Current established.
-          level == kModifyLdtLock ||
-          // Threads are unregistered while holding the thread list lock, during this process they
-          // no longer exist and so we expect an unlock with no self.
-          level == kThreadListLock ||
-          // Ignore logging which may or may not have set up thread data structures.
-          level == kLoggingLock ||
-          // When transitioning from suspended to runnable, a daemon thread might be in
-          // a situation where the runtime is shutting down. To not crash our debug locking
-          // mechanism we just pass null Thread* to the MutexLock during that transition
-          // (see Thread::TransitionFromSuspendedToRunnable).
-          level == kThreadSuspendCountLock ||
-          // Avoid recursive death.
-          level == kAbortLock ||
-          // Locks at the absolute top of the stack can be locked at any time.
-          level == kTopLockLevel ||
-          // The unexpected signal handler may be catching signals from any thread.
-          level == kUnexpectedSignalLock) << level;
-  }
+  CHECK(!Locks::IsSafeToCallAbortRacy() ||
+        // Used during thread creation to avoid races with runtime shutdown. Thread::Current not
+        // yet established.
+        level == kRuntimeShutdownLock ||
+        // Thread Ids are allocated/released before threads are established.
+        level == kAllocatedThreadIdsLock ||
+        // Thread LDT's are initialized without Thread::Current established.
+        level == kModifyLdtLock ||
+        // Threads are unregistered while holding the thread list lock, during this process they
+        // no longer exist and so we expect an unlock with no self.
+        level == kThreadListLock ||
+        // Ignore logging which may or may not have set up thread data structures.
+        level == kLoggingLock ||
+        // When transitioning from suspended to runnable, a daemon thread might be in
+        // a situation where the runtime is shutting down. To not crash our debug locking
+        // mechanism we just pass null Thread* to the MutexLock during that transition
+        // (see Thread::TransitionFromSuspendedToRunnable).
+        level == kThreadSuspendCountLock ||
+        // Avoid recursive death.
+        level == kAbortLock ||
+        // Locks at the absolute top of the stack can be locked at any time.
+        level == kTopLockLevel ||
+        // The unexpected signal handler may be catching signals from any thread.
+        level == kUnexpectedSignalLock)
+      << level;
 }
 
-inline void BaseMutex::RegisterAsLocked(Thread* self) {
+inline void BaseMutex::RegisterAsLocked(Thread* self, bool check) {
   if (UNLIKELY(self == nullptr)) {
-    CheckUnattachedThread(level_);
-    return;
+    if (check) {
+      CheckUnattachedThread(level_);
+    }
+  } else {
+    RegisterAsLockedImpl(self, level_, check);
   }
-  RegisterAsLockedImpl(self, level_);
 }
 
-inline void BaseMutex::RegisterAsLockedImpl(Thread* self, LockLevel level) {
+inline void BaseMutex::RegisterAsLockedImpl(Thread* self, LockLevel level, bool check) {
   DCHECK(self != nullptr);
   DCHECK_EQ(level_, level);
   // It would be nice to avoid this condition checking in the non-debug case,
@@ -107,7 +108,7 @@ inline void BaseMutex::RegisterAsLockedImpl(Thread* self, LockLevel level) {
   if (UNLIKELY(level == kThreadWaitLock) && self->GetHeldMutex(kThreadWaitLock) != nullptr) {
     level = kThreadWaitWakeLock;
   }
-  if (kDebugLocking) {
+  if (check) {
     // Check if a bad Mutex of this level or lower is held.
     bool bad_mutexes_held = false;
     // Specifically allow a kTopLockLevel lock to be gained when the current thread holds the
@@ -161,10 +162,12 @@ inline void BaseMutex::RegisterAsLockedImpl(Thread* self, LockLevel level) {
 
 inline void BaseMutex::RegisterAsUnlocked(Thread* self) {
   if (UNLIKELY(self == nullptr)) {
-    CheckUnattachedThread(level_);
-    return;
+    if (kDebugLocking) {
+      CheckUnattachedThread(level_);
+    }
+  } else {
+    RegisterAsUnlockedImpl(self, level_);
   }
-  RegisterAsUnlockedImpl(self , level_);
 }
 
 inline void BaseMutex::RegisterAsUnlockedImpl(Thread* self, LockLevel level) {
@@ -306,7 +309,7 @@ inline void MutatorMutex::TransitionFromRunnableToSuspended(Thread* self) {
 }
 
 inline void MutatorMutex::TransitionFromSuspendedToRunnable(Thread* self) {
-  RegisterAsLockedImpl(self, kMutatorLock);
+  RegisterAsLockedImpl(self, kMutatorLock, kDebugLocking);
   AssertSharedHeld(self);
 }