Bug 1208687: Only discard events from the outermost queue. r=ehsan
☠☠ backed out by fbcf840b4b21 ☠ ☠
authorKyle Huey <khuey@kylehuey.com>
Sun, 27 Sep 2015 21:57:36 -0700
changeset 264617 ee27fc2f6a1d4415e2a56b416260b77799149871
parent 264616 efcfe0c08c24ae9148838278c5cc9e41470a2272
child 264618 069515c93b965a2a854623005991e96dacd1bada
push id65696
push userkhuey@mozilla.com
push dateMon, 28 Sep 2015 05:00:26 +0000
treeherdermozilla-inbound@ee27fc2f6a1d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1208687, 914762
milestone44.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 1208687: Only discard events from the outermost queue. r=ehsan When workers shut down we discard the event queue rather than running it to completion. Originally workers managed their event queue themselves and would simply iterate through the array of events and cancel them all. After bug 914762 this was done by setting a (thread-)global "canceling" flag and then calling NS_ProcessPendingEvents. But this neglects that a shut down request can be received while the worker is in a sync queue. In this case, calling NS_ProcessPendingEvents will process any events pending in the sync queue, which is *not* the queue we need to cancel. The fix is, if we are in a sync queue when NotifyInternal is called, to defer clearing the queue until the top-most sync queue is destroyed and we are about to return to the regular event queue. Only then can we call NS_ProcessPendingEvents to clear out the queue. Because we can never process any events from this queue while sync queues are active, the timing of the mass cancellation is unchanged from the perspective of events in the regular queue.
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -570,16 +570,23 @@ private:
 
     event->SetTrusted(true);
 
     globalScope->DispatchDOMEvent(nullptr, event, nullptr, nullptr);
 
     return true;
   }
 
+  NS_IMETHOD Cancel() override
+  {
+    // We need to run regardless.
+    Run();
+    return WorkerRunnable::Cancel();
+  }
+
   virtual void
   PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
           override
   {
     // Report errors.
     WorkerRunnable::PostRun(aCx, aWorkerPrivate, aRunResult);
 
     // Match the busy count increase from NotifyRunnable.
@@ -1225,16 +1232,23 @@ private:
   {
     if (mTimer) {
       mTimer->Cancel();
       mTimer = nullptr;
     }
 
     return true;
   }
+
+  NS_IMETHOD Cancel() override
+  {
+    // We need to run regardless.
+    Run();
+    return WorkerRunnable::Cancel();
+  }
 };
 
 class UpdateRuntimeOptionsRunnable final : public WorkerControlRunnable
 {
   JS::RuntimeOptions mRuntimeOptions;
 
 public:
   UpdateRuntimeOptionsRunnable(
@@ -3818,16 +3832,17 @@ WorkerPrivate::WorkerPrivate(JSContext* 
   , mErrorHandlerRecursionCount(0)
   , mNextTimeoutId(1)
   , mStatus(Pending)
   , mFrozen(false)
   , mTimerRunning(false)
   , mRunningExpiredTimeouts(false)
   , mCloseHandlerStarted(false)
   , mCloseHandlerFinished(false)
+  , mPendingEventQueueClearing(false)
   , mMemoryReporterRunning(false)
   , mBlockedForMemoryReporter(false)
   , mCancelAllPendingRunnables(false)
   , mPeriodicGCTimerRunning(false)
   , mIdleGCTimerRunning(false)
   , mWorkerScriptExecutedSuccessfully(false)
 {
   MOZ_ASSERT_IF(!IsDedicatedWorker(), !aSharedWorkerName.IsVoid());
@@ -4646,16 +4661,17 @@ WorkerPrivate::IsOnCurrentThread(bool* a
 }
 
 void
 WorkerPrivate::ScheduleDeletion(WorkerRanOrNot aRanOrNot)
 {
   AssertIsOnWorkerThread();
   MOZ_ASSERT(mChildWorkers.IsEmpty());
   MOZ_ASSERT(mSyncLoopStack.IsEmpty());
+  MOZ_ASSERT(!mPendingEventQueueClearing);
 
   ClearMainEventQueue(aRanOrNot);
 #ifdef DEBUG
   if (WorkerRan == aRanOrNot) {
     nsIThread* currentThread = NS_GetCurrentThread();
     MOZ_ASSERT(currentThread);
     MOZ_ASSERT(!NS_HasPendingEvents(currentThread));
   }
@@ -4876,16 +4892,17 @@ WorkerPrivate::ProcessAllControlRunnable
   return result;
 }
 
 void
 WorkerPrivate::ClearMainEventQueue(WorkerRanOrNot aRanOrNot)
 {
   AssertIsOnWorkerThread();
 
+  MOZ_ASSERT(!mSyncLoopStack.Length());
   MOZ_ASSERT(!mCancelAllPendingRunnables);
   mCancelAllPendingRunnables = true;
 
   if (WorkerNeverRan == aRanOrNot) {
     for (uint32_t count = mPreStartRunnables.Length(), index = 0;
          index < count;
          index++) {
       nsRefPtr<WorkerRunnable> runnable = mPreStartRunnables[index].forget();
@@ -5238,16 +5255,21 @@ WorkerPrivate::DestroySyncLoop(uint32_t 
 #endif
 
     // This will delete |loopInfo|!
     mSyncLoopStack.RemoveElementAt(aLoopIndex);
   }
 
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(aThread->PopEventQueue(nestedEventTarget)));
 
+  if (!mSyncLoopStack.Length() && mPendingEventQueueClearing) {
+    ClearMainEventQueue(WorkerRan);
+    mPendingEventQueueClearing = false;
+  }
+
   return result;
 }
 
 void
 WorkerPrivate::StopSyncLoop(nsIEventTarget* aSyncLoopTarget, bool aResult)
 {
   AssertIsOnWorkerThread();
   AssertValidSyncLoop(aSyncLoopTarget);
@@ -5504,17 +5526,23 @@ WorkerPrivate::NotifyInternal(JSContext*
   MOZ_ASSERT(previousStatus >= Canceling || mKillTime.IsNull());
 
   // Let all our features know the new status.
   NotifyFeatures(aCx, aStatus);
 
   // If this is the first time our status has changed then we need to clear the
   // main event queue.
   if (previousStatus == Running) {
-    ClearMainEventQueue(WorkerRan);
+    // NB: If we're in a sync loop, we can't clear the queue immediately,
+    // because this is the wrong queue. So we have to defer it until later.
+    if (mSyncLoopStack.Length()) {
+      mPendingEventQueueClearing = true;
+    } else {
+      ClearMainEventQueue(WorkerRan);
+    }
   }
 
   // If we've run the close handler, we don't need to do anything else.
   if (mCloseHandlerFinished) {
     return true;
   }
 
   // If the worker script never ran, or failed to compile, we don't need to do
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -939,16 +939,17 @@ class WorkerPrivate : public WorkerPriva
   uint32_t mErrorHandlerRecursionCount;
   uint32_t mNextTimeoutId;
   Status mStatus;
   bool mFrozen;
   bool mTimerRunning;
   bool mRunningExpiredTimeouts;
   bool mCloseHandlerStarted;
   bool mCloseHandlerFinished;
+  bool mPendingEventQueueClearing;
   bool mMemoryReporterRunning;
   bool mBlockedForMemoryReporter;
   bool mCancelAllPendingRunnables;
   bool mPeriodicGCTimerRunning;
   bool mIdleGCTimerRunning;
   bool mWorkerScriptExecutedSuccessfully;
   bool mPreferences[WORKERPREF_COUNT];
   bool mOnLine;