Bug 1615072 - don't trigger the deadlock detector from static initializers for RWLock; r=mccr8
authorNathan Froyd <froydnj@mozilla.com>
Fri, 14 Feb 2020 21:44:05 +0000
changeset 514160 4b0bed287f1a11fd828d76413a6fa4a82f67c46d
parent 514159 937ce62849e029f09054cdd50ddcf5855de65c71
child 514161 0809a778c694a05a11cfd999bb508e386cb04fee
push id37125
push usershindli@mozilla.com
push dateSat, 15 Feb 2020 09:56:17 +0000
treeherdermozilla-central@02b1aa498dd2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1615072
milestone75.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 1615072 - don't trigger the deadlock detector from static initializers for RWLock; r=mccr8 This change is a little gross, because we don't totally control where the statically initialized instances of `RWLock` live, due to its uses in third-party libraries. Differential Revision: https://phabricator.services.mozilla.com/D62936
config/external/rlbox/rlbox_config.h
xpcom/threads/RWLock.h
--- a/config/external/rlbox/rlbox_config.h
+++ b/config/external/rlbox/rlbox_config.h
@@ -10,26 +10,25 @@
 
 // RLBox uses c++17's shared_locks by default, even for the noop_sandbox
 // However c++17 shared_lock is not supported on macOS 10.9 to 10.11
 // Thus we use Firefox's shared lock implementation
 // This can be removed if macOS 10.9 to 10.11 support is dropped
 #  include "mozilla/RWLock.h"
 namespace rlbox {
 struct rlbox_shared_lock {
-  mozilla::RWLock rwlock;
-  rlbox_shared_lock() : rwlock("rlbox") {}
+  mozilla::detail::StaticRWLock rwlock;
 };
 }  // namespace rlbox
 #  define RLBOX_USE_CUSTOM_SHARED_LOCK
 #  define RLBOX_SHARED_LOCK(name) rlbox::rlbox_shared_lock name
 #  define RLBOX_ACQUIRE_SHARED_GUARD(name, ...) \
-    mozilla::AutoReadLock name((__VA_ARGS__).rwlock)
+    mozilla::detail::StaticAutoReadLock name((__VA_ARGS__).rwlock)
 #  define RLBOX_ACQUIRE_UNIQUE_GUARD(name, ...) \
-    mozilla::AutoWriteLock name((__VA_ARGS__).rwlock)
+    mozilla::detail::StaticAutoWriteLock name((__VA_ARGS__).rwlock)
 
 #endif
 
 // All uses are on the main thread right now, disable rlbox thread checks for
 // performance
 #define RLBOX_SINGLE_THREADED_INVOCATIONS
 
 // The MingW compiler does not correctly handle static thread_local inline
--- a/xpcom/threads/RWLock.h
+++ b/xpcom/threads/RWLock.h
@@ -85,55 +85,119 @@ class RWLock : public BlockingResourceBa
 #endif
 
 #ifdef DEBUG
   // We record the owning thread for write locks only.
   PRThread* mOwningThread;
 #endif
 };
 
-// Read lock and unlock a RWLock with RAII semantics.  Much preferred to bare
-// calls to ReadLock() and ReadUnlock().
-class MOZ_RAII AutoReadLock final {
+template <typename T>
+class MOZ_RAII BaseAutoReadLock {
  public:
-  explicit AutoReadLock(RWLock& aLock MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+  explicit BaseAutoReadLock(T& aLock MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : mLock(&aLock) {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
     MOZ_ASSERT(mLock, "null lock");
     mLock->ReadLock();
   }
 
-  ~AutoReadLock() { mLock->ReadUnlock(); }
+  ~BaseAutoReadLock() { mLock->ReadUnlock(); }
 
  private:
-  AutoReadLock() = delete;
-  AutoReadLock(const AutoReadLock&) = delete;
-  AutoReadLock& operator=(const AutoReadLock&) = delete;
+  BaseAutoReadLock() = delete;
+  BaseAutoReadLock(const BaseAutoReadLock&) = delete;
+  BaseAutoReadLock& operator=(const BaseAutoReadLock&) = delete;
 
-  RWLock* mLock;
+  T* mLock;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
-// Write lock and unlock a RWLock with RAII semantics.  Much preferred to bare
-// calls to WriteLock() and WriteUnlock().
-class MOZ_RAII AutoWriteLock final {
+template <typename T>
+class MOZ_RAII BaseAutoWriteLock final {
  public:
-  explicit AutoWriteLock(RWLock& aLock MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+  explicit BaseAutoWriteLock(T& aLock MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : mLock(&aLock) {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
     MOZ_ASSERT(mLock, "null lock");
     mLock->WriteLock();
   }
 
-  ~AutoWriteLock() { mLock->WriteUnlock(); }
+  ~BaseAutoWriteLock() { mLock->WriteUnlock(); }
 
  private:
-  AutoWriteLock() = delete;
-  AutoWriteLock(const AutoWriteLock&) = delete;
-  AutoWriteLock& operator=(const AutoWriteLock&) = delete;
+  BaseAutoWriteLock() = delete;
+  BaseAutoWriteLock(const BaseAutoWriteLock&) = delete;
+  BaseAutoWriteLock& operator=(const BaseAutoWriteLock&) = delete;
 
-  RWLock* mLock;
+  T* mLock;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+// Read lock and unlock a RWLock with RAII semantics.  Much preferred to bare
+// calls to ReadLock() and ReadUnlock().
+typedef BaseAutoReadLock<RWLock> AutoReadLock;
+
+// Write lock and unlock a RWLock with RAII semantics.  Much preferred to bare
+// calls to WriteLock() and WriteUnlock().
+typedef BaseAutoWriteLock<RWLock> AutoWriteLock;
+
+// XXX: normally we would define StaticRWLock as
+// MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS, but the contexts in which it
+// is used (e.g. member variables in a third-party library) are non-trivial
+// to modify to properly declare everything at static scope.  As those
+// third-party libraries are the only clients, put it behind the detail
+// namespace to discourage other (possibly erroneous) uses from popping up.
+
+namespace detail {
+
+class StaticRWLock {
+ public:
+  // In debug builds, check that mLock is initialized for us as we expect by
+  // the compiler.  In non-debug builds, don't declare a constructor so that
+  // the compiler can see that the constructor is trivial.
+#ifdef DEBUG
+  StaticRWLock() { MOZ_ASSERT(!mLock); }
+#endif
+
+  void ReadLock() { Lock()->ReadLock(); }
+  void ReadUnlock() { Lock()->ReadUnlock(); }
+  void WriteLock() { Lock()->WriteLock(); }
+  void WriteUnlock() { Lock()->WriteUnlock(); }
+
+ private:
+  RWLock* Lock() {
+    if (mLock) {
+      return mLock;
+    }
+
+    RWLock* lock = new RWLock("StaticRWLock");
+    if (!mLock.compareExchange(nullptr, lock)) {
+      delete lock;
+    }
+
+    return mLock;
+  }
+
+  Atomic<RWLock*> mLock;
+
+  // Disallow copy constructor, but only in debug mode.  We only define
+  // a default constructor in debug mode (see above); if we declared
+  // this constructor always, the compiler wouldn't generate a trivial
+  // default constructor for us in non-debug mode.
+#ifdef DEBUG
+  StaticRWLock(const StaticRWLock& aOther);
+#endif
+
+  // Disallow these operators.
+  StaticRWLock& operator=(StaticRWLock* aRhs);
+  static void* operator new(size_t) noexcept(true);
+  static void operator delete(void*);
+};
+
+typedef BaseAutoReadLock<StaticRWLock> StaticAutoReadLock;
+typedef BaseAutoWriteLock<StaticRWLock> StaticAutoWriteLock;
+
+}  // namespace detail
+
 }  // namespace mozilla
 
 #endif  // mozilla_RWLock_h