Bug 1038854 - Avoid making unlocked mutex to locked uncontended after recreating threads from pthread condition wait wrappers. r=khuey
authorTing-Yu Chou <janus926@gmail.com>
Thu, 31 Jul 2014 10:47:30 +0800
changeset 219893 f73cd738c1fefc3d698e6a8130bbf90231b6a0d8
parent 219892 299094e66ea07ec2b5057b99e26b2b41836ad48b
child 219894 e27aee35d0bf9be7f0963f6aa0f764cb5c7f74c8
push id583
push userbhearsum@mozilla.com
push dateMon, 24 Nov 2014 19:04:58 +0000
treeherdermozilla-release@c107e74250f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1038854
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1038854 - Avoid making unlocked mutex to locked uncontended after recreating threads from pthread condition wait wrappers. r=khuey
mozglue/build/Nuwa.cpp
--- a/mozglue/build/Nuwa.cpp
+++ b/mozglue/build/Nuwa.cpp
@@ -135,17 +135,31 @@ struct thread_info : public mozilla::Lin
 
   // The thread specific function to recreate the new thread. It's executed
   // after the thread is recreated.
   void (*recrFunc)(void *arg);
   void *recrArg;
 
   TLSInfoList tlsInfo;
 
-  pthread_mutex_t *reacquireMutex;
+  /**
+   * We must ensure that the recreated thread has entered pthread_cond_wait() or
+   * similar functions before proceeding to recreate the next one. Otherwise, if
+   * the next thread depends on the same mutex, it may be used in an incorrect
+   * state.  To do this, the main thread must unconditionally acquire the mutex.
+   * The mutex is unconditionally released when the recreated thread enters
+   * pthread_cond_wait().  The recreated thread may have locked the mutex itself
+   * (if the pthread_mutex_trylock succeeded) or another thread may have already
+   * held the lock.  If the recreated thread did lock the mutex we must balance
+   * that with another unlock on the main thread, which is signaled by
+   * condMutexNeedsBalancing.
+   */
+  pthread_mutex_t *condMutex;
+  bool condMutexNeedsBalancing;
+
   void *stk;
 
   pid_t origNativeThreadID;
   pid_t recreatedNativeThreadID;
   char nativeThreadName[NATIVE_THREAD_NAME_LENGTH];
 };
 
 typedef struct thread_info thread_info_t;
@@ -496,17 +510,18 @@ static thread_info_t *
 thread_info_new(void) {
   /* link tinfo to sAllThreads */
   thread_info_t *tinfo = new thread_info_t();
   tinfo->flags = 0;
   tinfo->recrFunc = nullptr;
   tinfo->recrArg = nullptr;
   tinfo->recreatedThreadID = 0;
   tinfo->recreatedNativeThreadID = 0;
-  tinfo->reacquireMutex = nullptr;
+  tinfo->condMutex = nullptr;
+  tinfo->condMutexNeedsBalancing = false;
   tinfo->stk = MozTaggedAnonymousMmap(nullptr,
                                       NUWA_STACK_SIZE + getPageSize(),
                                       PROT_READ | PROT_WRITE,
                                       MAP_PRIVATE | MAP_ANONYMOUS,
                                       /* fd */ -1,
                                       /* offset */ 0,
                                       "nuwa-thread-stack");
 
@@ -1011,23 +1026,26 @@ extern "C" MFBT_API int
   THREAD_FREEZE_POINT1_VIP();
   if (freezePoint2) {
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
     RECREATE_GATE_VIP();
     return rv;
   }
   if (recreated && mtx) {
-    if (!freezePoint1 && pthread_mutex_trylock(mtx)) {
+    if (!freezePoint1) {
+      tinfo->condMutex = mtx;
       // The thread was frozen in pthread_cond_wait() after releasing mtx in the
       // Nuwa process. In recreating this thread, We failed to reacquire mtx
       // with the pthread_mutex_trylock() call, that is, mtx was acquired by
       // another thread. Because of this, we need the main thread's help to
       // reacquire mtx so that it will be in a valid state.
-      tinfo->reacquireMutex = mtx;
+      if (!pthread_mutex_trylock(mtx)) {
+        tinfo->condMutexNeedsBalancing = true;
+      }
     }
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
   }
   rv = REAL(pthread_cond_wait)(cond, mtx);
   if (recreated && mtx) {
     // We still need to be gated as not to acquire another mutex associated with
     // another VIP thread and interfere with it.
@@ -1047,18 +1065,21 @@ extern "C" MFBT_API int
   THREAD_FREEZE_POINT1_VIP();
   if (freezePoint2) {
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
     RECREATE_GATE_VIP();
     return rv;
   }
   if (recreated && mtx) {
-    if (!freezePoint1 && pthread_mutex_trylock(mtx)) {
-      tinfo->reacquireMutex = mtx;
+    if (!freezePoint1) {
+      tinfo->condMutex = mtx;
+      if (!pthread_mutex_trylock(mtx)) {
+        tinfo->condMutexNeedsBalancing = true;
+      }
     }
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
   }
   rv = REAL(pthread_cond_timedwait)(cond, mtx, abstime);
   if (recreated && mtx) {
     RECREATE_GATE_VIP();
   }
@@ -1082,18 +1103,21 @@ extern "C" MFBT_API int
   THREAD_FREEZE_POINT1_VIP();
   if (freezePoint2) {
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
     RECREATE_GATE_VIP();
     return rv;
   }
   if (recreated && mtx) {
-    if (!freezePoint1 && pthread_mutex_trylock(mtx)) {
-      tinfo->reacquireMutex = mtx;
+    if (!freezePoint1) {
+      tinfo->condMutex = mtx;
+      if (!pthread_mutex_trylock(mtx)) {
+        tinfo->condMutexNeedsBalancing = true;
+      }
     }
     RECREATE_CONTINUE();
     RECREATE_PASS_VIP();
   }
   rv = REAL(__pthread_cond_timedwait)(cond, mtx, abstime, clock);
   if (recreated && mtx) {
     RECREATE_GATE_VIP();
   }
@@ -1398,18 +1422,22 @@ RecreateThreads() {
   pthread_mutex_unlock(&sThreadCountLock);
 
   RECREATE_START();
   while (tinfo != nullptr) {
     if (tinfo->flags & TINFO_FLAG_NUWA_SUPPORT) {
       RECREATE_BEFORE(tinfo);
       thread_recreate(tinfo);
       RECREATE_WAIT();
-      if (tinfo->reacquireMutex) {
-        REAL(pthread_mutex_lock)(tinfo->reacquireMutex);
+      if (tinfo->condMutex) {
+        // Synchronize with the recreated thread in pthread_cond_wait().
+        REAL(pthread_mutex_lock)(tinfo->condMutex);
+        if (tinfo->condMutexNeedsBalancing) {
+          pthread_mutex_unlock(tinfo->condMutex);
+        }
       }
     } else if(!(tinfo->flags & TINFO_FLAG_NUWA_SKIP)) {
       // An unmarked thread is found other than the main thread.
 
       // All threads should be marked as one of SUPPORT or SKIP, or
       // abort the process to make sure all threads in the Nuwa
       // process are Nuwa-aware.
       abort();