Bug 1370644 - Part 1: Only use NS_ERROR for an imminent failure. r=froydnj, a=debug-only
authorEric Rahm <erahm@mozilla.com>
Fri, 09 Jun 2017 16:09:17 -0700
changeset 413934 a5658a3fd394183d08992e38035ae2bf47e85426
parent 413933 22f9d84db6dd080de3ee2ecd484a3caa926f5b7e
child 413935 ddde9a8077529ebeacc1f3abb0f8bc6096b05b3e
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, debug-only
bugs1370644
milestone55.0
Bug 1370644 - Part 1: Only use NS_ERROR for an imminent failure. r=froydnj, a=debug-only This modifies the logic in |CheckAcquisition| to only call |NS_ERROR| if we're really going to deadlock. Instead, if we detect a suspicious cycle, we just use an |NS_WARNING|. This means that we'll still output warning text in debug builds, but we won't cause the process to abort. MozReview-Commit-ID: 71mFInWwbDY
xpcom/tests/gtest/TestDeadlockDetector.cpp
xpcom/threads/BlockingResourceBase.cpp
--- a/xpcom/tests/gtest/TestDeadlockDetector.cpp
+++ b/xpcom/tests/gtest/TestDeadlockDetector.cpp
@@ -131,16 +131,18 @@ TEST_F(TESTNAME(DeadlockDetectorTest), T
         "--- Next dependency:.*--- Mutex : dd.sanity2.m2.*"
         "=== Cycle completed at.*--- Mutex : dd.sanity2.m1.*"
         "###!!! Deadlock may happen NOW!.*" // better catch these easy cases...
         "###!!! ASSERTION: Potential deadlock detected.*";
 
     ASSERT_DEATH_IF_SUPPORTED(Sanity2_Child(), regex);
 }
 
+#if 0
+// Temporarily disabled, see bug 1370644.
 int
 Sanity3_Child()
 {
     DisableCrashReporter();
 
     MUTEX m1("dd.sanity3.m1");
     MUTEX m2("dd.sanity3.m2");
     MUTEX m3("dd.sanity3.m3");
@@ -168,16 +170,17 @@ TEST_F(TESTNAME(DeadlockDetectorTest), T
         "--- Next dependency:.*--- Mutex : dd.sanity3.m2.*"
         "--- Next dependency:.*--- Mutex : dd.sanity3.m3.*"
         "--- Next dependency:.*--- Mutex : dd.sanity3.m4.*"
         "=== Cycle completed at.*--- Mutex : dd.sanity3.m1.*"
         "###!!! ASSERTION: Potential deadlock detected.*";
 
     ASSERT_DEATH_IF_SUPPORTED(Sanity3_Child(), regex);
 }
+#endif
 
 int
 Sanity4_Child()
 {
     DisableCrashReporter();
 
     mozilla::ReentrantMonitor m1("dd.sanity4.m1");
     MUTEX m2("dd.sanity4.m2");
@@ -214,16 +217,18 @@ struct ThreadState
   const nsTArray<MUTEX*>& locks;
 
   /**
    * Integer argument used to identify each thread.
    */
   int id;
 };
 
+#if 0
+// Temporarily disabled, see bug 1370644.
 static void
 TwoThreads_thread(void* arg)
 {
     ThreadState* state = static_cast<ThreadState*>(arg);
 
     MUTEX* ttM1 = state->locks[0];
     MUTEX* ttM2 = state->locks[1];
 
@@ -272,16 +277,17 @@ TEST_F(TESTNAME(DeadlockDetectorTest), T
         "###!!! ERROR: Potential deadlock detected.*"
         "=== Cyclical dependency starts at.*--- Mutex : dd.twothreads.m2.*"
         "--- Next dependency:.*--- Mutex : dd.twothreads.m1.*"
         "=== Cycle completed at.*--- Mutex : dd.twothreads.m2.*"
         "###!!! ASSERTION: Potential deadlock detected.*";
 
     ASSERT_DEATH_IF_SUPPORTED(TwoThreads_Child(), regex);
 }
+#endif
 
 static void
 ContentionNoDeadlock_thread(void* arg)
 {
     const uint32_t K = 100000;
 
     ThreadState* state = static_cast<ThreadState*>(arg);
     int32_t starti = static_cast<int32_t>(state->id);
--- a/xpcom/threads/BlockingResourceBase.cpp
+++ b/xpcom/threads/BlockingResourceBase.cpp
@@ -296,21 +296,22 @@ BlockingResourceBase::CheckAcquire()
     fputs("\n###!!! Deadlock may happen NOW!\n\n", stderr);
     out.AppendLiteral("\n###!!! Deadlock may happen NOW!\n\n");
   } else {
     fputs("\nDeadlock may happen for some other execution\n\n",
           stderr);
     out.AppendLiteral("\nDeadlock may happen for some other execution\n\n");
   }
 
-  // XXX can customize behavior on whether we /think/ deadlock is
-  // XXX about to happen.  for example:
-  // XXX   if (maybeImminent)
-  //           NS_RUNTIMEABORT(out.get());
-  NS_ERROR(out.get());
+  // Only error out if we think a deadlock is imminent.
+  if (maybeImminent) {
+    NS_ERROR(out.get());
+  } else {
+    NS_WARNING(out.get());
+  }
 }
 
 
 void
 BlockingResourceBase::Acquire()
 {
   if (mType == eCondVar) {
     NS_NOTYETIMPLEMENTED(