commit 79400aa5124f89daec47d3cf7c6c38030547a770
author Alex Light <allight@google.com> Tue Jul 18 15:34:21 2017 -0700
committer Alex Light <allight@google.com> Wed Jul 19 08:35:34 2017 -0700
tree 280b6f2c77ce4a76bd8f196579499064fc19d711
parent 527f18fcebb8f5799bad483b8164fe35ce3688f2

Whitelist user_code_suspension_lock_ for ConditionVariable wait

ConditionVariable::Wait checks to make sure the current lock status is
sane. We need to whitelist the user_code_suspension_lock_ for being
held through the wait when it is a threads resume_cond_ since this
lock needs to be held through the entirety of user-code suspension.

We also add a check to make sure the thread will not deadlock.

Bug: 63813338
Test: stress --cpu 64 & \
      while ./test/run-test --host --jit 1904; do; done

Change-Id: I24577b66629279106de328aad1afd191725c94fb

runtime/base/mutex.cc

diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index a472b67..03dda12 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -233,8 +233,27 @@
     for (int i = kLockLevelCount - 1; i >= 0; --i) {
       if (i != level_) {
         BaseMutex* held_mutex = self->GetHeldMutex(static_cast<LockLevel>(i));
-        // We expect waits to happen while holding the thread list suspend thread lock.
-        if (held_mutex != nullptr) {
+        // We allow the thread to wait even if the user_code_suspension_lock_ is held so long as we
+        // are some thread's resume_cond_ (level_ == kThreadSuspendCountLock). This just means that
+        // gc or some other internal process is suspending the thread while it is trying to suspend
+        // some other thread. So long as the current thread is not being suspended by a
+        // SuspendReason::kForUserCode (which needs the user_code_suspension_lock_ to clear) this is
+        // fine.
+        if (held_mutex == Locks::user_code_suspension_lock_ && level_ == kThreadSuspendCountLock) {
+          // No thread safety analysis is fine since we have both the user_code_suspension_lock_
+          // from the line above and the ThreadSuspendCountLock since it is our level_. We use this
+          // lambda to avoid having to annotate the whole function as NO_THREAD_SAFETY_ANALYSIS.
+          auto is_suspending_for_user_code = [self]() NO_THREAD_SAFETY_ANALYSIS {
+            return self->GetUserCodeSuspendCount() != 0;
+          };
+          if (is_suspending_for_user_code()) {
+            LOG(ERROR) << "Holding \"" << held_mutex->name_ << "\" "
+                      << "(level " << LockLevel(i) << ") while performing wait on "
+                      << "\"" << name_ << "\" (level " << level_ << ") "
+                      << "with SuspendReason::kForUserCode pending suspensions";
+            bad_mutexes_held = true;
+          }
+        } else if (held_mutex != nullptr) {
           LOG(ERROR) << "Holding \"" << held_mutex->name_ << "\" "
                      << "(level " << LockLevel(i) << ") while performing wait on "
                      << "\"" << name_ << "\" (level " << level_ << ")";
@@ -243,7 +262,7 @@
       }
     }
     if (gAborting == 0) {  // Avoid recursive aborts.
-      CHECK(!bad_mutexes_held);
+      CHECK(!bad_mutexes_held) << this;
     }
   }
 }