Bug 604026 - Sync event loops in workers should be created only if compatible with the worker shutdown status

7 files changed, 46 insertions, 20 deletions

diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp
index ad95d4896e..e1910536f2 100644
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -587,7 +587,7 @@ ContentSecurityPolicyAllows(JSContext* aCx)
         new LogViolationDetailsRunnable(worker, fileName, lineNum);
 
     ErrorResult rv;
-    runnable->Dispatch(rv);
+    runnable->Dispatch(Killing, rv);
     if (NS_WARN_IF(rv.Failed())) {
       rv.SuppressException();
     }
diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp
index 46545e737d..56b18441e5 100644
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -2118,12 +2118,16 @@ LoadAllScripts(WorkerPrivate* aWorkerPrivate,
   aWorkerPrivate->AssertIsOnWorkerThread();
   NS_ASSERTION(!aLoadInfos.IsEmpty(), "Bad arguments!");
 
-  AutoSyncLoopHolder syncLoop(aWorkerPrivate);
+  AutoSyncLoopHolder syncLoop(aWorkerPrivate, Terminating);
+  nsCOMPtr<nsIEventTarget> syncLoopTarget = syncLoop.GetEventTarget();
+  if (!syncLoopTarget) {
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return;
+  }
 
   RefPtr<ScriptLoaderRunnable> loader =
-    new ScriptLoaderRunnable(aWorkerPrivate, syncLoop.EventTarget(),
-                             aLoadInfos, aIsMainScript, aWorkerScriptType,
-                             aRv);
+    new ScriptLoaderRunnable(aWorkerPrivate, syncLoopTarget, aLoadInfos,
+                             aIsMainScript, aWorkerScriptType, aRv);
 
   NS_ASSERTION(aLoadInfos.IsEmpty(), "Should have swapped!");
 
@@ -2184,7 +2188,7 @@ ChannelFromScriptURLWorkerThread(JSContext* aCx,
     new ChannelGetterRunnable(aParent, aScriptURL, aChannel);
 
   ErrorResult rv;
-  getter->Dispatch(rv);
+  getter->Dispatch(Terminating, rv);
   if (rv.Failed()) {
     NS_ERROR("Failed to dispatch!");
     return rv.StealNSResult();
diff --git a/dom/workers/WorkerNavigator.cpp b/dom/workers/WorkerNavigator.cpp
index 682c7a22ca..f798968815 100644
--- a/dom/workers/WorkerNavigator.cpp
+++ b/dom/workers/WorkerNavigator.cpp
@@ -152,7 +152,7 @@ WorkerNavigator::GetUserAgent(nsString& aUserAgent, ErrorResult& aRv) const
   RefPtr<GetUserAgentRunnable> runnable =
     new GetUserAgentRunnable(workerPrivate, aUserAgent);
 
-  runnable->Dispatch(aRv);
+  runnable->Dispatch(Terminating, aRv);
 }
 
 uint64_t
diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
index 8848e881a7..612090027a 100644
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -5446,10 +5446,18 @@ WorkerPrivate::CancelAllTimeouts()
 }
 
 already_AddRefed<nsIEventTarget>
-WorkerPrivate::CreateNewSyncLoop()
+WorkerPrivate::CreateNewSyncLoop(Status aFailStatus)
 {
   AssertIsOnWorkerThread();
 
+  {
+    MutexAutoLock lock(mMutex);
+
+    if (mStatus >= aFailStatus) {
+      return nullptr;
+    }
+  }
+
   nsCOMPtr<nsIThreadInternal> thread = do_QueryInterface(NS_GetCurrentThread());
   MOZ_ASSERT(thread);
 
diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h
index 28283bed78..20a530205d 100644
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -1442,8 +1442,11 @@ private:
     memcpy(aPreferences, mPreferences, WORKERPREF_COUNT * sizeof(bool));
   }
 
+  // If the worker shutdown status is equal or greater then aFailStatus, this
+  // operation will fail and nullptr will be returned. See WorkerHolder.h for
+  // more information about the correct value to use.
   already_AddRefed<nsIEventTarget>
-  CreateNewSyncLoop();
+  CreateNewSyncLoop(Status aFailStatus);
 
   bool
   RunCurrentSyncLoop();
@@ -1518,9 +1521,11 @@ class AutoSyncLoopHolder
   uint32_t mIndex;
 
 public:
-  explicit AutoSyncLoopHolder(WorkerPrivate* aWorkerPrivate)
+  // See CreateNewSyncLoop() for more information about the correct value to use
+  // for aFailStatus.
+  AutoSyncLoopHolder(WorkerPrivate* aWorkerPrivate, Status aFailStatus)
   : mWorkerPrivate(aWorkerPrivate)
-  , mTarget(aWorkerPrivate->CreateNewSyncLoop())
+  , mTarget(aWorkerPrivate->CreateNewSyncLoop(aFailStatus))
   , mIndex(aWorkerPrivate->mSyncLoopStack.Length() - 1)
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
@@ -1528,7 +1533,7 @@ public:
 
   ~AutoSyncLoopHolder()
   {
-    if (mWorkerPrivate) {
+    if (mWorkerPrivate && mTarget) {
       mWorkerPrivate->AssertIsOnWorkerThread();
       mWorkerPrivate->StopSyncLoop(mTarget, false);
       mWorkerPrivate->DestroySyncLoop(mIndex);
@@ -1547,8 +1552,9 @@ public:
   }
 
   nsIEventTarget*
-  EventTarget() const
+  GetEventTarget() const
   {
+    // This can be null if CreateNewSyncLoop() fails.
     return mTarget;
   }
 };
diff --git a/dom/workers/WorkerRunnable.cpp b/dom/workers/WorkerRunnable.cpp
index 1e16d72546..6bbe40f66a 100644
--- a/dom/workers/WorkerRunnable.cpp
+++ b/dom/workers/WorkerRunnable.cpp
@@ -568,15 +568,20 @@ WorkerMainThreadRunnable::WorkerMainThreadRunnable(WorkerPrivate* aWorkerPrivate
 }
 
 void
-WorkerMainThreadRunnable::Dispatch(ErrorResult& aRv)
+WorkerMainThreadRunnable::Dispatch(Status aFailStatus, ErrorResult& aRv)
 {
   mWorkerPrivate->AssertIsOnWorkerThread();
 
   TimeStamp startTime = TimeStamp::NowLoRes();
 
-  AutoSyncLoopHolder syncLoop(mWorkerPrivate);
+  AutoSyncLoopHolder syncLoop(mWorkerPrivate, aFailStatus);
 
-  mSyncLoopTarget = syncLoop.EventTarget();
+  mSyncLoopTarget = syncLoop.GetEventTarget();
+  if (!mSyncLoopTarget) {
+    // SyncLoop creation can fail if the worker is shutting down.
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return;
+  }
 
   DebugOnly<nsresult> rv = mWorkerPrivate->DispatchToMainThread(this);
   MOZ_ASSERT(NS_SUCCEEDED(rv),
@@ -621,7 +626,7 @@ bool
 WorkerCheckAPIExposureOnMainThreadRunnable::Dispatch()
 {
   ErrorResult rv;
-  WorkerMainThreadRunnable::Dispatch(rv);
+  WorkerMainThreadRunnable::Dispatch(Terminating, rv);
   bool ok = !rv.Failed();
   rv.SuppressException();
   return ok;
diff --git a/dom/workers/WorkerRunnable.h b/dom/workers/WorkerRunnable.h
index c65060f448..8249a8053e 100644
--- a/dom/workers/WorkerRunnable.h
+++ b/dom/workers/WorkerRunnable.h
@@ -401,9 +401,12 @@ protected:
 
 public:
   // Dispatch the runnable to the main thread.  If dispatch to main thread
-  // fails, or if the worker is shut down while dispatching, an error will be
-  // reported on aRv.  In that case the error MUST be propagated out to script.
-  void Dispatch(ErrorResult& aRv);
+  // fails, or if the worker is in a state equal or greater of aFailStatus, an
+  // error will be reported on aRv. Normally you want to use 'Terminating' for
+  // aFailStatus, except if you want an infallible runnable. In this case, use
+  // 'Killing'.
+  // In that case the error MUST be propagated out to script.
+  void Dispatch(Status aFailStatus, ErrorResult& aRv);
 
 private:
   NS_IMETHOD Run() override;