Bug 1003730 - Always terminate workers when in canceling state, r=mrbkap
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 18 Dec 2018 12:48:11 +0100
changeset 451096 2d4a6ccb481186b2a1ee498c1c07a7f9370aa7e3
parent 451095 e5910e32045729328863efdf1e06b72675589e54
child 451097 faac12c6cb6159bc3e07e20fd53de140becabcc4
push id110603
push useramarchesini@mozilla.com
push dateTue, 18 Dec 2018 11:48:33 +0000
treeherdermozilla-inbound@2d4a6ccb4811 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs1003730
milestone66.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 1003730 - Always terminate workers when in canceling state, r=mrbkap
dom/workers/WorkerPrivate.cpp
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -2986,32 +2986,47 @@ void WorkerPrivate::ShutdownGCTimers() {
   data->mGCTimer = nullptr;
   data->mPeriodicGCTimerRunning = false;
   data->mIdleGCTimerRunning = false;
 }
 
 bool WorkerPrivate::InterruptCallback(JSContext* aCx) {
   MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
 
+  // If we are here it's because a WorkerControlRunnable has been dispatched.
+  // The runnable could be processed here or it could have already been
+  // processed by a sync event loop.
+  // The most important thing this method must do, is to decide if the JS
+  // execution should continue or not. If the runnable returns an error or if
+  // the worker status is >= Canceling, we should stop the JS execution.
+
   MOZ_ASSERT(!JS_IsExceptionPending(aCx));
 
   bool mayContinue = true;
   bool scheduledIdleGC = false;
 
   for (;;) {
     // Run all control events now.
     auto result = ProcessAllControlRunnables();
     if (result == ProcessAllControlRunnablesResult::Abort) {
       mayContinue = false;
     }
 
     bool mayFreeze = data->mFrozen;
-    if (mayFreeze) {
+
+    {
       MutexAutoLock lock(mMutex);
-      mayFreeze = mStatus <= Running;
+
+      if (mayFreeze) {
+        mayFreeze = mStatus <= Running;
+      }
+
+      if (mStatus >= Canceling) {
+        mayContinue = false;
+      }
     }
 
     if (!mayContinue || !mayFreeze) {
       break;
     }
 
     // Cancel the periodic GC timer here before freezing. The idle GC timer
     // will clean everything up once it runs.
@@ -3505,20 +3520,21 @@ bool WorkerPrivate::RunCurrentSyncLoop()
       for (;;) {
         while (mControlQueue.IsEmpty() && !normalRunnablesPending &&
                !(normalRunnablesPending = NS_HasPendingEvents(mThread))) {
           WaitForWorkerEvents();
         }
 
         auto result = ProcessAllControlRunnablesLocked();
         if (result != ProcessAllControlRunnablesResult::Nothing) {
-          // XXXkhuey how should we handle Abort here? See Bug 1003730.
-
-          // The state of the world may have changed. Recheck it.
-          normalRunnablesPending = NS_HasPendingEvents(mThread);
+          // The state of the world may have changed. Recheck it if we need to
+          // continue.
+          normalRunnablesPending =
+              result == ProcessAllControlRunnablesResult::MayContinue &&
+              NS_HasPendingEvents(mThread);
 
           // NB: If we processed a NotifyRunnable, we might have run
           // non-control runnables, one of which may have shut down the
           // sync loop.
           if (loopInfo->mCompleted) {
             break;
           }
         }