Bug 1347963 - part 1 - introduce mozilla::RecursiveMutex; r=erahm
authorNathan Froyd <froydnj@mozilla.com>
Tue, 14 Mar 2017 14:05:51 -0400
changeset 420020 6c1c1d869619ba3b721630a1f6375a4d54697e31
parent 420019 cec8d814aa8326322ce3c47bc5321d6092fb5655
child 420021 7232578cc2b11d4348fb6bfa928005c4b264974b
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1347963
milestone56.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 1347963 - part 1 - introduce mozilla::RecursiveMutex; r=erahm Having a proper recursively-acquirable mutex type makes intent clearer, and RecursiveMutex also happens to be somewhat faster than ReentrantMonitor.
storage/test/gtest/test_deadlock_detector.cpp
xpcom/tests/gtest/TestDeadlockDetector.cpp
xpcom/tests/gtest/TestRecursiveMutex.cpp
xpcom/tests/gtest/TestSlicedInputStream.cpp
xpcom/tests/gtest/moz.build
xpcom/threads/BlockingResourceBase.cpp
xpcom/threads/BlockingResourceBase.h
xpcom/threads/RecursiveMutex.cpp
xpcom/threads/RecursiveMutex.h
xpcom/threads/ReentrantMonitor.h
xpcom/threads/moz.build
--- a/storage/test/gtest/test_deadlock_detector.cpp
+++ b/storage/test/gtest/test_deadlock_detector.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Note: This file is essentially a copy of
 // xpcom/tests/gtest/TestDeadlockDetector.cpp, but all mutexes were turned into
 // SQLiteMutexes. We use #include and some macros to avoid actual source code
 // duplication.
 
 #include "mozilla/CondVar.h"
+#include "mozilla/RecursiveMutex.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "SQLiteMutex.h"
 
 #include "gtest/gtest.h"
 
 using namespace mozilla;
 
 /**
--- a/xpcom/tests/gtest/TestDeadlockDetector.cpp
+++ b/xpcom/tests/gtest/TestDeadlockDetector.cpp
@@ -7,16 +7,17 @@
 #include "mozilla/ArrayUtils.h"
 
 #include "prthread.h"
 
 #include "nsTArray.h"
 #include "nsMemory.h"
 
 #include "mozilla/CondVar.h"
+#include "mozilla/RecursiveMutex.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "mozilla/Mutex.h"
 
 #ifdef MOZ_CRASHREPORTER
 #include "nsCOMPtr.h"
 #include "nsICrashReporter.h"
 #include "nsServiceManagerUtils.h"
 #endif
@@ -197,16 +198,41 @@ TEST_F(TESTNAME(DeadlockDetectorTest), T
         "###!!! ERROR: Potential deadlock detected.*"
         "=== Cyclical dependency starts at.*--- ReentrantMonitor : dd.sanity4.m1.*"
         "--- Next dependency:.*--- Mutex : dd.sanity4.m2.*"
         "=== Cycle completed at.*--- ReentrantMonitor : dd.sanity4.m1.*"
         "###!!! ASSERTION: Potential deadlock detected.*";
     ASSERT_DEATH_IF_SUPPORTED(Sanity4_Child(), regex);
 }
 
+int
+Sanity5_Child()
+{
+    DisableCrashReporter();
+
+    mozilla::RecursiveMutex m1("dd.sanity4.m1");
+    MUTEX m2("dd.sanity4.m2");
+    m1.Lock();
+    m2.Lock();
+    m1.Lock();
+    return 0;
+}
+
+TEST_F(TESTNAME(DeadlockDetectorTest), TESTNAME(Sanity5DeathTest))
+{
+    const char* const regex =
+        "Re-entering RecursiveMutex after acquiring other resources.*"
+        "###!!! ERROR: Potential deadlock detected.*"
+        "=== Cyclical dependency starts at.*--- RecursiveMutex : dd.sanity4.m1.*"
+        "--- Next dependency:.*--- Mutex : dd.sanity4.m2.*"
+        "=== Cycle completed at.*--- RecursiveMutex : dd.sanity4.m1.*"
+        "###!!! ASSERTION: Potential deadlock detected.*";
+    ASSERT_DEATH_IF_SUPPORTED(Sanity5_Child(), regex);
+}
+
 //-----------------------------------------------------------------------------
 // Multithreaded tests
 
 /**
  * Helper for passing state to threads in the multithread tests.
  */
 struct ThreadState
 {
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/gtest/TestRecursiveMutex.cpp
@@ -0,0 +1,25 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsThreadUtils.h"
+#include "mozilla/RecursiveMutex.h"
+#include "gtest/gtest.h"
+
+using mozilla::RecursiveMutex;
+using mozilla::RecursiveMutexAutoLock;
+
+// Basic test to make sure the underlying implementation of RecursiveMutex is,
+// well, actually recursively acquirable.
+
+TEST(RecursiveMutex, SmokeTest)
+{
+  RecursiveMutex mutex("testing mutex");
+
+  RecursiveMutexAutoLock lock1(mutex);
+  RecursiveMutexAutoLock lock2(mutex);
+
+  //...and done.
+}
--- a/xpcom/tests/gtest/TestSlicedInputStream.cpp
+++ b/xpcom/tests/gtest/TestSlicedInputStream.cpp
@@ -1,12 +1,13 @@
 #include "gtest/gtest.h"
 
 #include "nsCOMPtr.h"
 #include "nsIInputStream.h"
+#include "nsIPipe.h"
 #include "nsStreamUtils.h"
 #include "nsString.h"
 #include "nsStringStream.h"
 #include "SlicedInputStream.h"
 #include "Helpers.h"
 
 // This helper class is used to call OnInputStreamReady with the right stream
 // as argument.
--- a/xpcom/tests/gtest/moz.build
+++ b/xpcom/tests/gtest/moz.build
@@ -27,16 +27,17 @@ UNIFIED_SOURCES += [
     'TestNsDeque.cpp',
     'TestNSPRLogModulesParser.cpp',
     'TestObserverArray.cpp',
     'TestObserverService.cpp',
     'TestPipes.cpp',
     'TestPLDHash.cpp',
     'TestPriorityQueue.cpp',
     'TestRacingServiceManager.cpp',
+    'TestRecursiveMutex.cpp',
     'TestRWLock.cpp',
     'TestSlicedInputStream.cpp',
     'TestSnappyStreams.cpp',
     'TestStateWatching.cpp',
     'TestStorageStream.cpp',
     'TestStrings.cpp',
     'TestStringStream.cpp',
     'TestSynchronization.cpp',
--- a/xpcom/threads/BlockingResourceBase.cpp
+++ b/xpcom/threads/BlockingResourceBase.cpp
@@ -15,16 +15,17 @@
 #include "CodeAddressService.h"
 #include "nsHashKeys.h"
 #include "mozilla/StackWalk.h"
 #include "nsTHashtable.h"
 #endif
 
 #include "mozilla/CondVar.h"
 #include "mozilla/DeadlockDetector.h"
+#include "mozilla/RecursiveMutex.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/RWLock.h"
 
 #if defined(MOZILLA_INTERNAL_API)
 #include "GeckoProfiler.h"
 #endif //MOZILLA_INTERNAL_API
 
@@ -33,17 +34,17 @@
 namespace mozilla {
 //
 // BlockingResourceBase implementation
 //
 
 // static members
 const char* const BlockingResourceBase::kResourceTypeName[] = {
   // needs to be kept in sync with BlockingResourceType
-  "Mutex", "ReentrantMonitor", "CondVar"
+  "Mutex", "ReentrantMonitor", "CondVar", "RecursiveMutex"
 };
 
 #ifdef DEBUG
 
 PRCallOnceType BlockingResourceBase::sCallOnce;
 unsigned BlockingResourceBase::sResourceAcqnChainFrontTPI = (unsigned)-1;
 BlockingResourceBase::DDT* BlockingResourceBase::sDeadlockDetector;
 
@@ -523,16 +524,76 @@ ReentrantMonitor::Wait(PRIntervalTime aI
   SetAcquisitionState(savedAcquisitionState);
   mChainPrev = savedChainPrev;
 
   return rv;
 }
 
 
 //
+// Debug implementation of RecursiveMutex
+void
+RecursiveMutex::Lock()
+{
+  BlockingResourceBase* chainFront = ResourceChainFront();
+
+  // the code below implements mutex reentrancy semantics
+
+  if (this == chainFront) {
+    // immediately re-entered the mutex: acceptable
+    LockInternal();
+    ++mEntryCount;
+    return;
+  }
+
+  // this is sort of a hack around not recording the thread that
+  // owns this monitor
+  if (chainFront) {
+    for (BlockingResourceBase* br = ResourceChainPrev(chainFront);
+         br;
+         br = ResourceChainPrev(br)) {
+      if (br == this) {
+        NS_WARNING(
+          "Re-entering RecursiveMutex after acquiring other resources.");
+
+        // show the caller why this is potentially bad
+        CheckAcquire();
+
+        LockInternal();
+        ++mEntryCount;
+        return;
+      }
+    }
+  }
+
+  CheckAcquire();
+  LockInternal();
+  NS_ASSERTION(mEntryCount == 0, "RecursiveMutex isn't free!");
+  Acquire();       // protected by us
+  mOwningThread = PR_GetCurrentThread();
+  mEntryCount = 1;
+}
+
+void
+RecursiveMutex::Unlock()
+{
+  if (--mEntryCount == 0) {
+    Release();              // protected by us
+    mOwningThread = nullptr;
+  }
+  UnlockInternal();
+}
+
+void
+RecursiveMutex::AssertCurrentThreadIn()
+{
+  MOZ_ASSERT(IsAcquired() && mOwningThread == PR_GetCurrentThread());
+}
+
+//
 // Debug implementation of CondVar
 nsresult
 CondVar::Wait(PRIntervalTime aInterval)
 {
   AssertCurrentThreadOwnsMutex();
 
   // save mutex state and reset to empty
   AcquisitionState savedAcquisitionState = mLock->GetAcquisitionState();
--- a/xpcom/threads/BlockingResourceBase.h
+++ b/xpcom/threads/BlockingResourceBase.h
@@ -45,17 +45,17 @@ template <class T> class DeadlockDetecto
  * BlockingResourceBase
  * Base class of resources that might block clients trying to acquire them.
  * Does debugging and deadlock detection in DEBUG builds.
  **/
 class BlockingResourceBase
 {
 public:
   // Needs to be kept in sync with kResourceTypeNames.
-  enum BlockingResourceType { eMutex, eReentrantMonitor, eCondVar };
+  enum BlockingResourceType { eMutex, eReentrantMonitor, eCondVar, eRecursiveMutex };
 
   /**
    * kResourceTypeName
    * Human-readable version of BlockingResourceType enum.
    */
   static const char* const kResourceTypeName[];
 
 
new file mode 100644
--- /dev/null
+++ b/xpcom/threads/RecursiveMutex.cpp
@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/RecursiveMutex.h"
+
+#ifdef XP_WIN
+#include <windows.h>
+
+#define NativeHandle(m) (reinterpret_cast<CRITICAL_SECTION*>(&m))
+#endif
+
+namespace mozilla {
+
+RecursiveMutex::RecursiveMutex(const char* aName MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
+  : BlockingResourceBase(aName, eRecursiveMutex)
+#ifdef DEBUG
+  , mOwningThread(nullptr)
+  , mEntryCount(0)
+#endif
+{
+  MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+#ifdef XP_WIN
+  // This number was adapted from NSPR.
+  static const DWORD sLockSpinCount = 100;
+
+#if defined(RELEASE_OR_BETA)
+  // Vista and later automatically allocate and subsequently leak a debug info
+  // object for each critical section that we allocate unless we tell the
+  // system not to do that.
+  DWORD flags = CRITICAL_SECTION_NO_DEBUG_INFO;
+#else
+  DWORD flags = 0;
+#endif
+  BOOL r = InitializeCriticalSectionEx(NativeHandle(mMutex),
+                                       sLockSpinCount, flags);
+  MOZ_RELEASE_ASSERT(r);
+#else
+  pthread_mutexattr_t attr;
+
+  MOZ_RELEASE_ASSERT(pthread_mutexattr_init(&attr) == 0,
+                     "pthread_mutexattr_init failed");
+
+  MOZ_RELEASE_ASSERT(pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) == 0,
+                     "pthread_mutexattr_settype failed");
+
+  MOZ_RELEASE_ASSERT(pthread_mutex_init(&mMutex, &attr) == 0,
+                     "pthread_mutex_init failed");
+
+  MOZ_RELEASE_ASSERT(pthread_mutexattr_destroy(&attr) == 0,
+                     "pthread_mutexattr_destroy failed");
+#endif
+}
+
+RecursiveMutex::~RecursiveMutex()
+{
+#ifdef XP_WIN
+  DeleteCriticalSection(NativeHandle(mMutex));
+#else
+  MOZ_RELEASE_ASSERT(pthread_mutex_destroy(&mMutex) == 0,
+                     "pthread_mutex_destroy failed");
+#endif
+}
+
+void
+RecursiveMutex::LockInternal()
+{
+#ifdef XP_WIN
+  EnterCriticalSection(NativeHandle(mMutex));
+#else
+  MOZ_RELEASE_ASSERT(pthread_mutex_lock(&mMutex) == 0,
+                     "pthread_mutex_lock failed");
+#endif
+}
+
+void
+RecursiveMutex::UnlockInternal()
+{
+#ifdef XP_WIN
+  LeaveCriticalSection(NativeHandle(mMutex));
+#else
+  MOZ_RELEASE_ASSERT(pthread_mutex_unlock(&mMutex) == 0,
+                     "pthread_mutex_unlock failed");
+#endif
+}
+
+} // namespace mozilla
new file mode 100644
--- /dev/null
+++ b/xpcom/threads/RecursiveMutex.h
@@ -0,0 +1,124 @@
+
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// A lock that can be acquired multiple times on the same thread.
+
+#ifndef mozilla_RecursiveMutex_h
+#define mozilla_RecursiveMutex_h
+
+#include "mozilla/BlockingResourceBase.h"
+#include "mozilla/GuardObjects.h"
+
+#ifndef XP_WIN
+#include <pthread.h>
+#endif
+
+namespace mozilla {
+
+class RecursiveMutex : public BlockingResourceBase
+{
+public:
+  explicit RecursiveMutex(const char* aName MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
+  ~RecursiveMutex();
+
+#ifdef DEBUG
+  void Lock();
+  void Unlock();
+#else
+  void Lock() { LockInternal(); }
+  void Unlock() { UnlockInternal(); }
+#endif
+
+  void AssertCurrentThreadIn()
+#ifdef DEBUG
+    ;
+#else
+  {
+  }
+#endif
+
+private:
+  RecursiveMutex() = delete;
+  RecursiveMutex(const RecursiveMutex&) = delete;
+  RecursiveMutex& operator=(const RecursiveMutex&) = delete;
+
+  void LockInternal();
+  void UnlockInternal();
+
+#ifdef DEBUG
+  PRThread* mOwningThread;
+  size_t mEntryCount;
+#endif
+
+#if !defined(XP_WIN)
+  pthread_mutex_t mMutex;
+#else
+  // We eschew including windows.h and using CRITICAL_SECTION here so that files
+  // including us don't also pull in windows.h.  Just use a type that's big
+  // enough for CRITICAL_SECTION, and we'll fix it up later.
+  void* mMutex[6];
+#endif
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
+class MOZ_RAII RecursiveMutexAutoLock
+{
+public:
+  explicit RecursiveMutexAutoLock(RecursiveMutex& aRecursiveMutex
+                                  MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    : mRecursiveMutex(&aRecursiveMutex)
+  {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    NS_ASSERTION(mRecursiveMutex, "null mutex");
+    mRecursiveMutex->Lock();
+  }
+
+  ~RecursiveMutexAutoLock(void)
+  {
+    mRecursiveMutex->Unlock();
+  }
+
+private:
+  RecursiveMutexAutoLock() = delete;
+  RecursiveMutexAutoLock(const RecursiveMutexAutoLock&) = delete;
+  RecursiveMutexAutoLock& operator=(const RecursiveMutexAutoLock&) = delete;
+  static void* operator new(size_t) CPP_THROW_NEW;
+
+  mozilla::RecursiveMutex* mRecursiveMutex;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
+class MOZ_RAII RecursiveMutexAutoUnlock
+{
+public:
+  explicit RecursiveMutexAutoUnlock(RecursiveMutex& aRecursiveMutex
+                                    MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    : mRecursiveMutex(&aRecursiveMutex)
+  {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    NS_ASSERTION(mRecursiveMutex, "null mutex");
+    mRecursiveMutex->Unlock();
+  }
+
+  ~RecursiveMutexAutoUnlock(void)
+  {
+    mRecursiveMutex->Lock();
+  }
+
+private:
+  RecursiveMutexAutoUnlock() = delete;
+  RecursiveMutexAutoUnlock(const RecursiveMutexAutoUnlock&) = delete;
+  RecursiveMutexAutoUnlock& operator=(const RecursiveMutexAutoUnlock&) = delete;
+  static void* operator new(size_t) CPP_THROW_NEW;
+
+  mozilla::RecursiveMutex* mRecursiveMutex;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
+} // namespace mozilla
+
+#endif // mozilla_RecursiveMutex_h
--- a/xpcom/threads/ReentrantMonitor.h
+++ b/xpcom/threads/ReentrantMonitor.h
@@ -251,10 +251,9 @@ private:
   ReentrantMonitorAutoExit& operator=(const ReentrantMonitorAutoExit&);
   static void* operator new(size_t) CPP_THROW_NEW;
 
   ReentrantMonitor* mReentrantMonitor;
 };
 
 } // namespace mozilla
 
-
 #endif // ifndef mozilla_ReentrantMonitor_h
--- a/xpcom/threads/moz.build
+++ b/xpcom/threads/moz.build
@@ -44,16 +44,17 @@ EXPORTS.mozilla += [
     'HangAnnotations.h',
     'HangMonitor.h',
     'InputEventStatistics.h',
     'LazyIdleThread.h',
     'MainThreadIdlePeriod.h',
     'Monitor.h',
     'MozPromise.h',
     'Mutex.h',
+    'RecursiveMutex.h',
     'ReentrantMonitor.h',
     'RWLock.h',
     'SchedulerGroup.h',
     'SharedThreadPool.h',
     'StateMirroring.h',
     'StateWatching.h',
     'SyncRunnable.h',
     'SystemGroup.h',
@@ -77,16 +78,17 @@ UNIFIED_SOURCES += [
     'nsMemoryPressure.cpp',
     'nsProcessCommon.cpp',
     'nsProxyRelease.cpp',
     'nsThread.cpp',
     'nsThreadManager.cpp',
     'nsThreadPool.cpp',
     'nsThreadUtils.cpp',
     'nsTimerImpl.cpp',
+    'RecursiveMutex.cpp',
     'RWLock.cpp',
     'SchedulerGroup.cpp',
     'SharedThreadPool.cpp',
     'SystemGroup.cpp',
     'TaskQueue.cpp',
     'ThreadStackHelper.cpp',
     'ThrottledEventQueue.cpp',
     'TimerThread.cpp',