Bug 1483561 - Notify the next lock owner only when the last owner is about to release it, r=froydnj.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 29 Aug 2018 16:42:32 -1000
changeset 482370 85e1ddee9e8f5421f9834f406027857413cfdf4f
parent 482368 653e1282f4ce8acfa3329cb4b0c0595d8356ca00
child 482371 fc437d64c83157d3539c8b337d4048e00cd373ea
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersfroydnj
bugs1483561
milestone63.0a1
Bug 1483561 - Notify the next lock owner only when the last owner is about to release it, r=froydnj.
toolkit/recordreplay/Lock.cpp
toolkit/recordreplay/Lock.h
toolkit/recordreplay/ProcessRedirectDarwin.cpp
--- a/toolkit/recordreplay/Lock.cpp
+++ b/toolkit/recordreplay/Lock.cpp
@@ -146,17 +146,17 @@ Lock::Find(void* aNativeLock)
       return iter->second;
     }
   }
 
   return nullptr;
 }
 
 void
-Lock::Enter(const std::function<void()>& aCallback)
+Lock::Enter()
 {
   MOZ_RELEASE_ASSERT(!AreThreadEventsPassedThrough() && !HasDivergedFromRecording());
   MOZ_RELEASE_ASSERT(!AreThreadEventsDisallowed());
 
   RecordReplayAssert("Lock %d", (int) mId);
 
   // Include an event in each thread's record when a lock acquire begins. This
   // is not required by the replay but is used to check that lock acquire order
@@ -169,19 +169,26 @@ Lock::Enter(const std::function<void()>&
   LockAcquires* acquires = gLockAcquires.Get(mId);
   if (IsRecording()) {
     acquires->mAcquires->WriteScalar(thread->Id());
   } else {
     // Wait until this thread is next in line to acquire the lock.
     while (thread->Id() != acquires->mNextOwner) {
       Thread::Wait();
     }
-    // Acquire the lock before updating the next owner.
-    aCallback();
-    acquires->ReadAndNotifyNextOwner(thread);
+  }
+}
+
+void
+Lock::Exit()
+{
+  if (IsReplaying()) {
+    // Notify the next owner before releasing the lock.
+    LockAcquires* acquires = gLockAcquires.Get(mId);
+    acquires->ReadAndNotifyNextOwner(Thread::Current());
   }
 }
 
 struct AtomicLock : public detail::MutexImpl
 {
   using detail::MutexImpl::lock;
   using detail::MutexImpl::unlock;
 };
--- a/toolkit/recordreplay/Lock.h
+++ b/toolkit/recordreplay/Lock.h
@@ -37,19 +37,22 @@ public:
     MOZ_ASSERT(aId);
   }
 
   size_t Id() { return mId; }
 
   // When recording, this is called after the lock has been acquired, and
   // records the acquire in the lock's acquire order stream. When replaying,
   // this is called before the lock has been acquired, and blocks the thread
-  // until it is next in line to acquire the lock before acquiring it via
-  // aCallback.
-  void Enter(const std::function<void()>& aCallback);
+  // until it is next in line to acquire the lock.
+  void Enter();
+
+  // This is called before releasing the lock, allowing the next owner to
+  // acquire it while replaying.
+  void Exit();
 
   // Create a new Lock corresponding to a native lock, with a fresh ID.
   static void New(void* aNativeLock);
 
   // Destroy any Lock associated with a native lock.
   static void Destroy(void* aNativeLock);
 
   // Get the recorded Lock for a native lock if there is one, otherwise null.
--- a/toolkit/recordreplay/ProcessRedirectDarwin.cpp
+++ b/toolkit/recordreplay/ProcessRedirectDarwin.cpp
@@ -1175,24 +1175,26 @@ WaitForCvar(pthread_mutex_t* aMutex, boo
   Lock* lock = Lock::Find(aMutex);
   if (!lock) {
     AutoEnsurePassThroughThreadEvents pt;
     return aCallback();
   }
   RecordReplayAssert("WaitForCvar %d", (int) lock->Id());
   ssize_t rv = 0;
   if (IsRecording()) {
-    {
-      AutoPassThroughThreadEvents pt;
-      rv = aCallback();
-    }
+    AutoPassThroughThreadEvents pt;
+    rv = aCallback();
   } else {
     DirectUnlockMutex(aMutex);
   }
-  lock->Enter([=]() { DirectLockMutex(aMutex); });
+  lock->Exit();
+  lock->Enter();
+  if (IsReplaying()) {
+    DirectLockMutex(aMutex);
+  }
   if (aRecordReturnValue) {
     return RecordReplayValue(rv);
   }
   MOZ_RELEASE_ASSERT(rv == 0);
   return 0;
 }
 
 static ssize_t
@@ -1268,21 +1270,30 @@ RR_pthread_mutex_destroy(pthread_mutex_t
 static ssize_t
 RR_pthread_mutex_lock(pthread_mutex_t* aMutex)
 {
   Lock* lock = Lock::Find(aMutex);
   if (!lock) {
     AutoEnsurePassThroughThreadEventsUseStackPointer pt;
     return OriginalCall(pthread_mutex_lock, ssize_t, aMutex);
   }
+  ssize_t rv = 0;
   if (IsRecording()) {
-    DirectLockMutex(aMutex);
+    AutoPassThroughThreadEvents pt;
+    rv = OriginalCall(pthread_mutex_lock, ssize_t, aMutex);
   }
-  lock->Enter([=]() { DirectLockMutex(aMutex); });
-  return 0;
+  rv = RecordReplayValue(rv);
+  MOZ_RELEASE_ASSERT(rv == 0 || rv == EDEADLK);
+  if (rv == 0) {
+    lock->Enter();
+    if (IsReplaying()) {
+      DirectLockMutex(aMutex);
+    }
+  }
+  return rv;
 }
 
 static ssize_t
 RR_pthread_mutex_trylock(pthread_mutex_t* aMutex)
 {
   Lock* lock = Lock::Find(aMutex);
   if (!lock) {
     AutoEnsurePassThroughThreadEvents pt;
@@ -1291,26 +1302,35 @@ RR_pthread_mutex_trylock(pthread_mutex_t
   ssize_t rv = 0;
   if (IsRecording()) {
     AutoPassThroughThreadEvents pt;
     rv = OriginalCall(pthread_mutex_trylock, ssize_t, aMutex);
   }
   rv = RecordReplayValue(rv);
   MOZ_RELEASE_ASSERT(rv == 0 || rv == EBUSY);
   if (rv == 0) {
-    lock->Enter([=]() { DirectLockMutex(aMutex); });
+    lock->Enter();
+    if (IsReplaying()) {
+      DirectLockMutex(aMutex);
+    }
   }
   return rv;
 }
 
 static ssize_t
 RR_pthread_mutex_unlock(pthread_mutex_t* aMutex)
 {
-  AutoEnsurePassThroughThreadEventsUseStackPointer pt;
-  return OriginalCall(pthread_mutex_unlock, ssize_t, aMutex);
+  Lock* lock = Lock::Find(aMutex);
+  if (!lock) {
+    AutoEnsurePassThroughThreadEventsUseStackPointer pt;
+    return OriginalCall(pthread_mutex_unlock, ssize_t, aMutex);
+  }
+  lock->Exit();
+  DirectUnlockMutex(aMutex);
+  return 0;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // stdlib redirections
 ///////////////////////////////////////////////////////////////////////////////
 
 static ssize_t
 RR_dlclose(void* aHandle)