Bug 1384395 - Use nsAutoOwningThread for mfbt/WeakPtr.h thread assertions (r=froydnj)
authorBill McCloskey <billm@mozilla.com>
Fri, 26 May 2017 14:21:28 -0700
changeset 420572 dd7b9bb53462d7ef12fb770003a70f98a9e987a5
parent 420571 7521180d3de0017d32bbd22f3bffe5060b45bdc4
child 420573 74b6145a2873a7c72efd011eb07881b255b7d960
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)
reviewersfroydnj
bugs1384395
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 1384395 - Use nsAutoOwningThread for mfbt/WeakPtr.h thread assertions (r=froydnj) MozReview-Commit-ID: DF4DiffL4Qq
mfbt/WeakPtr.h
xpcom/base/nsISupportsImpl.cpp
xpcom/base/nsISupportsImpl.h
--- a/mfbt/WeakPtr.h
+++ b/mfbt/WeakPtr.h
@@ -65,60 +65,58 @@
  */
 
 #ifndef mozilla_WeakPtr_h
 #define mozilla_WeakPtr_h
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/Maybe.h"
 #include "mozilla/RefCounted.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/TypeTraits.h"
 
 #include <string.h>
 
+#if defined(MOZILLA_INTERNAL_API)
+// For thread safety checking.
+#include "nsISupportsImpl.h"
+#endif
+
+#if defined(MOZILLA_INTERNAL_API) && defined(MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED)
+
 // Weak referencing is not implemeted as thread safe.  When a WeakPtr
 // is created or dereferenced on thread A but the real object is just
 // being Released() on thread B, there is a possibility of a race
 // when the proxy object (detail::WeakReference) is notified about
 // the real object destruction just between when thread A is storing
 // the object pointer locally and is about to add a reference to it.
 //
 // Hence, a non-null weak proxy object is considered to have a single
 // "owning thread".  It means that each query for a weak reference,
 // its dereference, and destruction of the real object must all happen
 // on a single thread.  The following macros implement assertions for
 // checking these conditions.
 //
-// We disable this on MinGW. MinGW has two threading models: win32
-// API based, which disables std::thread; and POSIX based which
-// enables it but requires an emulation library (winpthreads).
-// Rather than attempting to switch to pthread emulation at this point,
-// we are disabling the std::thread based assertion checking.
-//
-// In the future, to enable it we could
-// a. have libgcc/stdc++ support win32 threads natively
-// b. switch to POSIX-based threading in MinGW with pthread emulation
-// c. refactor it to not use std::thread
+// We re-use XPCOM's nsAutoOwningThread checks when they are available. This has
+// the advantage that it works with cooperative thread pools.
 
-#if !defined(__MINGW32__) && (defined(DEBUG) || (defined(NIGHTLY_BUILD) && !defined(MOZ_PROFILING)))
-
-#include <thread>
 #define MOZ_WEAKPTR_DECLARE_THREAD_SAFETY_CHECK \
-  std::thread::id _owningThread; \
-  bool _empty; // If it was initialized as a placeholder with mPtr = nullptr.
+  /* Will be none if mPtr = nullptr. */ \
+  Maybe<nsAutoOwningThread> _owningThread;
 #define MOZ_WEAKPTR_INIT_THREAD_SAFETY_CHECK() \
   do { \
-    _owningThread = std::this_thread::get_id(); \
-    _empty = !p; \
+    if (p) { \
+      _owningThread.emplace(); \
+    } \
   } while (false)
 #define MOZ_WEAKPTR_ASSERT_THREAD_SAFETY() \
   do { \
-    if (!(_empty || _owningThread == std::this_thread::get_id())) { \
+    if (_owningThread.isSome() && !_owningThread.ref().IsCurrentThread()) { \
       WeakPtrTraits<T>::AssertSafeToAccessFromNonOwningThread(); \
     } \
   } while (false)
 #define MOZ_WEAKPTR_ASSERT_THREAD_SAFETY_DELEGATED(that) \
   (that)->AssertThreadSafety();
 
 #define MOZ_WEAKPTR_THREAD_SAFETY_CHECKING 1
 
--- a/xpcom/base/nsISupportsImpl.cpp
+++ b/xpcom/base/nsISupportsImpl.cpp
@@ -33,14 +33,20 @@ NS_TableDrivenQI(void* aThis, REFNSIID a
 nsAutoOwningThread::nsAutoOwningThread()
   : mThread(GetCurrentVirtualThread())
 {
 }
 
 void
 nsAutoOwningThread::AssertCurrentThreadOwnsMe(const char* msg) const
 {
-  if (MOZ_UNLIKELY(mThread != GetCurrentVirtualThread())) {
+  if (MOZ_UNLIKELY(!IsCurrentThread())) {
     // `msg` is a string literal by construction.
     MOZ_CRASH_UNSAFE_OOL(msg);
   }
 }
+
+bool
+nsAutoOwningThread::IsCurrentThread() const
+{
+  return mThread == GetCurrentVirtualThread();
+}
 #endif
--- a/xpcom/base/nsISupportsImpl.h
+++ b/xpcom/base/nsISupportsImpl.h
@@ -64,16 +64,18 @@ public:
   // can then be assured that we effectively are passing a literal string
   // to MOZ_CRASH_UNSAFE_OOL.
   template<int N>
   void AssertOwnership(const char (&aMsg)[N]) const
   {
     AssertCurrentThreadOwnsMe(aMsg);
   }
 
+  bool IsCurrentThread() const;
+
 private:
   void AssertCurrentThreadOwnsMe(const char* aMsg) const;
 
   void* mThread;
 };
 
 #define NS_DECL_OWNINGTHREAD            nsAutoOwningThread _mOwningThread;
 #define NS_ASSERT_OWNINGTHREAD_AGGREGATE(agg, _class) \