Bug 1504638 - Put some of WorkerPrivate's members behind thread access guards r=asuth,baku,froydnj

Differential Revision: https://phabricator.services.mozilla.com/D10368

dom/workers/RuntimeService.cpp

@@ -1648,6 +1648,7 @@ RuntimeService::ScheduleWorker(WorkerPrivate* aWorkerPrivate)
     NS_WARNING("Could not set the thread's priority!");
   }
 
+  aWorkerPrivate->SetThread(thread);
   JSContext* cx = CycleCollectedJSContext::Get()->Context();
   nsCOMPtr<nsIRunnable> runnable =
     new WorkerThreadPrimaryRunnable(aWorkerPrivate, thread,
@@ -2716,20 +2717,20 @@ WorkerThreadPrimaryRunnable::Run()
       MOZ_ASSERT(aWorkerPrivate);
       MOZ_ASSERT(aThread);
 
-      mWorkerPrivate->SetThread(aThread);
+      mWorkerPrivate->SetWorkerPrivateInWorkerThread(aThread);
     }
 
     ~SetThreadHelper()
     {
       if (mWorkerPrivate) {
-        mWorkerPrivate->SetThread(nullptr);
+        mWorkerPrivate->ResetWorkerPrivateInWorkerThread();
       }
     }
 
     void Nullify()
     {
       MOZ_ASSERT(mWorkerPrivate);
-      mWorkerPrivate->SetThread(nullptr);
+      mWorkerPrivate->ResetWorkerPrivateInWorkerThread();
       mWorkerPrivate = nullptr;
     }
   };

dom/workers/WorkerPrivate.h

@@ -21,6 +21,7 @@
 #include "mozilla/dom/workerinternals/JSSettings.h"
 #include "mozilla/dom/workerinternals/Queue.h"
 #include "mozilla/PerformanceCounter.h"
+#include "mozilla/ThreadBound.h"
 
 class nsIConsoleReportCollector;
 class nsIThreadInternal;
@@ -402,15 +403,15 @@ public:
   WorkerGlobalScope*
   GlobalScope() const
   {
-    AssertIsOnWorkerThread();
-    return mScope;
+    MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
+    return data->mScope;
   }
 
   WorkerDebuggerGlobalScope*
   DebuggerGlobalScope() const
   {
-    AssertIsOnWorkerThread();
-    return mDebuggerScope;
+    MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
+    return data->mDebuggerScope;
   }
 
   nsICSPEventListener*
@@ -419,6 +420,12 @@ public:
   void
   SetThread(WorkerThread* aThread);
 
+  void
+  SetWorkerPrivateInWorkerThread(WorkerThread* aThread);
+
+  void
+  ResetWorkerPrivateInWorkerThread();
+
   bool
   IsOnWorkerThread() const;
 
@@ -472,8 +479,8 @@ public:
   bool
   OnLine() const
   {
-    AssertIsOnWorkerThread();
-    return mOnLine;
+    MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
+    return data->mOnLine;
   }
 
   void
@@ -1302,8 +1309,9 @@ private:
   bool
   HasActiveHolders()
   {
-    return !(mChildWorkers.IsEmpty() && mTimeouts.IsEmpty() &&
-             mHolders.IsEmpty());
+    MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
+    return !(data->mChildWorkers.IsEmpty() && data->mTimeouts.IsEmpty() &&
+             data->mHolders.IsEmpty());
   }
 
   class EventTarget;
@@ -1363,13 +1371,10 @@ private:
   RefPtr<WorkerThread> mThread;
   PRThread* mPRThread;
 
-  // Things touched on worker thread only.
-  RefPtr<WorkerGlobalScope> mScope;
-  RefPtr<WorkerDebuggerGlobalScope> mDebuggerScope;
-  nsTArray<WorkerPrivate*> mChildWorkers;
-  nsTObserverArray<WorkerHolder*> mHolders;
-  nsTArray<nsAutoPtr<TimeoutInfo>> mTimeouts;
+  // Accessed from main thread
   RefPtr<ThrottledEventQueue> mMainThreadEventTarget;
+
+  // Accessed from worker thread and destructing thread
   RefPtr<WorkerEventTarget> mWorkerControlEventTarget;
   RefPtr<WorkerEventTarget> mWorkerHybridEventTarget;
 
@@ -1394,15 +1399,8 @@ private:
   // modifications are done with mMutex held *only* in DEBUG builds.
   nsTArray<nsAutoPtr<SyncLoopInfo>> mSyncLoopStack;
 
-  nsCOMPtr<nsITimer> mTimer;
-  nsCOMPtr<nsITimerCallback> mTimerRunnable;
-
   nsCOMPtr<nsITimer> mCancelingTimer;
 
-  nsCOMPtr<nsITimer> mGCTimer;
-
-  RefPtr<MemoryReporter> mMemoryReporter;
-
   // fired on the main thread if the worker script fails to load
   nsCOMPtr<nsIRunnable> mLoadFailedRunnable;
 
@@ -1421,7 +1419,6 @@ private:
   TimeStamp mKillTime;
   WorkerStatus mParentStatus;
   WorkerStatus mStatus;
-  UniquePtr<ClientSource> mClientSource;
 
   // This is touched on parent thread only, but it can be read on a different
   // thread before crashing because hanging.
@@ -1433,26 +1430,48 @@ private:
   DOMHighResTimeStamp mCreationTimeHighRes;
 
   // Things touched on worker thread only.
-  uint32_t mNumHoldersPreventingShutdownStart;
-  uint32_t mDebuggerEventLoopLevel;
+  struct WorkerThreadAccessible
+  {
+    explicit WorkerThreadAccessible(WorkerPrivate* aParent);
 
-  uint32_t mErrorHandlerRecursionCount;
-  uint32_t mNextTimeoutId;
+    RefPtr<WorkerGlobalScope> mScope;
+    RefPtr<WorkerDebuggerGlobalScope> mDebuggerScope;
+    nsTArray<WorkerPrivate*> mChildWorkers;
+    nsTObserverArray<WorkerHolder*> mHolders;
+    nsTArray<nsAutoPtr<TimeoutInfo>> mTimeouts;
+
+    nsCOMPtr<nsITimer> mTimer;
+    nsCOMPtr<nsITimerCallback> mTimerRunnable;
+
+    nsCOMPtr<nsITimer> mGCTimer;
+
+    RefPtr<MemoryReporter> mMemoryReporter;
+
+    UniquePtr<ClientSource> mClientSource;
+
+    uint32_t mNumHoldersPreventingShutdownStart;
+    uint32_t mDebuggerEventLoopLevel;
+
+    uint32_t mErrorHandlerRecursionCount;
+    uint32_t mNextTimeoutId;
+
+    bool mFrozen;
+    bool mTimerRunning;
+    bool mRunningExpiredTimeouts;
+    bool mPeriodicGCTimerRunning;
+    bool mIdleGCTimerRunning;
+    bool mOnLine;
+  };
+  ThreadBound<WorkerThreadAccessible> mWorkerThreadAccessible;
 
   // SharedWorkers may have multiple windows paused, so this must be
   // a count instead of just a boolean.
   uint32_t mParentWindowPausedDepth;
 
-  bool mFrozen;
-  bool mTimerRunning;
-  bool mRunningExpiredTimeouts;
   bool mPendingEventQueueClearing;
   bool mCancelAllPendingRunnables;
-  bool mPeriodicGCTimerRunning;
-  bool mIdleGCTimerRunning;
   bool mWorkerScriptExecutedSuccessfully;
   bool mFetchHandlerWasAdded;
-  bool mOnLine;
   bool mMainThreadObjectsForgotten;
   bool mIsChromeWorker;
   bool mParentFrozen;

xpcom/threads/ThreadBound.h

@@ -0,0 +1,170 @@
+/* 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 class for values only accessible from a single designated thread.
+
+#ifndef mozilla_ThreadBound_h
+#define mozilla_ThreadBound_h
+
+#include "mozilla/Atomics.h"
+#include "prthread.h"
+
+#include <type_traits>
+
+namespace mozilla {
+
+template <typename T>
+class ThreadBound;
+
+namespace detail {
+
+template <bool Condition, typename T>
+struct AddConstIf
+{
+  using type = T;
+};
+
+template <typename T>
+struct AddConstIf<true, T>
+{
+  using type = typename std::add_const<T>::type;
+};
+
+}
+
+// A ThreadBound<T> is a T that can only be accessed by a specific
+// thread. To enforce this rule, the inner T is only accessible
+// through a non-copyable, immovable accessor object.
+// Given a ThreadBound<T> threadBoundData, it can be accessed like so:
+//
+//   MOZ_ACCESS_THREAD_BOUND(threadBoundData, innerData);
+//   innerData->DoStuff();
+//
+// Trying to access a ThreadBound<T> from a different thread will
+// trigger a MOZ_DIAGNOSTIC_ASSERT.
+//   The encapsulated T is constructed during the construction of the
+// enclosing ThreadBound<T> by forwarding all of the latter's
+// constructor parameters to the former. A newly constructed
+// ThreadBound<T> is bound to the thread it's constructed in. It's
+// possible to rebind the data to some otherThread by calling
+//
+//   threadBoundData.Transfer(otherThread);
+//
+// on the thread that threadBoundData is currently bound to, as long
+// as it's not currently being accessed. (Trying to rebind from
+// another thread or while an accessor exists will trigger an
+// assertion.)
+//
+// Note: A ThreadBound<T> may be destructed from any thread, not just
+// its designated thread at the time the destructor is invoked.
+template <typename T>
+class ThreadBound final
+{
+public:
+  template <typename... Args>
+  explicit ThreadBound(Args&&... aArgs)
+    : mData(std::forward<Args>(aArgs)...)
+    , mThread(PR_GetCurrentThread())
+    , mAccessCount(0)
+  {}
+
+  ~ThreadBound()
+  {
+    AssertIsNotCurrentlyAccessed();
+  }
+
+  void Transfer(const PRThread* const aDest)
+  {
+    AssertIsCorrectThread();
+    AssertIsNotCurrentlyAccessed();
+    mThread = aDest;
+  }
+
+private:
+  T mData;
+
+  // This member is (potentially) accessed by multiple threads and is
+  // thus the first point of synchronization between them.
+  Atomic<const PRThread*, ReleaseAcquire> mThread;
+
+  // In order to support nested accesses (e.g. from different stack
+  // frames) it's necessary to maintain a counter of the existing
+  // accessor. Since it's possible to access a const ThreadBound, the
+  // counter is mutable. It's atomic because accessing it synchronizes
+  // access to mData (see comment in Accessor's constructor).
+  using AccessCountType = Atomic<int, ReleaseAcquire>;
+  mutable AccessCountType mAccessCount;
+
+public:
+  template <bool IsConst>
+  class MOZ_STACK_CLASS Accessor final
+  {
+    using DataType = typename detail::AddConstIf<IsConst, T>::type;
+
+  public:
+    explicit Accessor(typename detail::AddConstIf<IsConst, ThreadBound>::type& aThreadBound)
+      : mData(aThreadBound.mData)
+      , mAccessCount(aThreadBound.mAccessCount)
+    {
+      aThreadBound.AssertIsCorrectThread();
+
+      // This load/store serves as a memory fence that guards mData
+      // against accesses that would trip the thread assertion.
+      // (Otherwise one of the loads in the caller's instruction
+      // stream might be scheduled before the assertion.)
+      ++mAccessCount;
+    }
+
+    Accessor(const Accessor&) = delete;
+    Accessor(Accessor&&) = delete;
+    Accessor& operator=(const Accessor&) = delete;
+    Accessor& operator=(Accessor&&) = delete;
+
+    ~Accessor()
+    {
+      --mAccessCount;
+    }
+
+    DataType* operator->()
+    {
+      return &mData;
+    }
+
+  private:
+    DataType& mData;
+    AccessCountType& mAccessCount;
+  };
+
+  template <typename U>
+  using AccessorFor = Accessor<std::is_const<typename std::remove_reference<U>::type>::value>;
+
+private:
+  bool IsCorrectThread() const
+  {
+    return mThread == PR_GetCurrentThread();
+  }
+
+  bool IsNotCurrentlyAccessed() const
+  {
+    return mAccessCount == 0;
+  }
+
+#define MOZ_DEFINE_THREAD_BOUND_ASSERT(predicate) \
+  void Assert ## predicate() const \
+  { \
+    MOZ_DIAGNOSTIC_ASSERT(predicate()); \
+  }
+
+  MOZ_DEFINE_THREAD_BOUND_ASSERT(IsCorrectThread)
+  MOZ_DEFINE_THREAD_BOUND_ASSERT(IsNotCurrentlyAccessed)
+
+#undef MOZ_DEFINE_THREAD_BOUND_ASSERT
+};
+
+#define MOZ_ACCESS_THREAD_BOUND(value, name) \
+  decltype(value)::AccessorFor<decltype(*&value)> name(value)
+
+}
+
+#endif // mozilla_ThreadBound_h

xpcom/threads/moz.build

@@ -67,6 +67,7 @@ EXPORTS.mozilla += [
     'TaskCategory.h',
     'TaskDispatcher.h',
     'TaskQueue.h',
+    'ThreadBound.h',
     'ThreadEventQueue.h',
     'ThrottledEventQueue.h',
 ]