Bug 1540080 - Execute the canceling runnable after self.close() even when we have sync event loops, r=asuth
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 02 Apr 2019 20:53:16 +0000
changeset 467735 b4a7aeb63c654d55f39d7ff2d56fd6d268f73928
parent 467734 2e3359dc72e8ae3cca9f3ce2300bed8d60540b50
child 467736 3134740d831cc24b5b931a8512584100fcc10471
push id82158
push useramarchesini@mozilla.com
push dateWed, 03 Apr 2019 10:47:45 +0000
treeherderautoland@b4a7aeb63c65 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1540080
milestone68.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 1540080 - Execute the canceling runnable after self.close() even when we have sync event loops, r=asuth Differential Revision: https://phabricator.services.mozilla.com/D25605
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -2058,18 +2058,18 @@ WorkerPrivate::WorkerPrivate(WorkerPriva
           new WorkerEventTarget(this, WorkerEventTarget::Behavior::Hybrid)),
       mParentStatus(Pending),
       mStatus(Pending),
       mBusyCount(0),
       mLoadingWorkerScript(false),
       mCreationTimeStamp(TimeStamp::Now()),
       mCreationTimeHighRes((double)PR_Now() / PR_USEC_PER_MSEC),
       mWorkerThreadAccessible(aParent),
+      mPostSyncLoopOperations(0),
       mParentWindowPaused(false),
-      mPendingEventQueueClearing(false),
       mCancelAllPendingRunnables(false),
       mWorkerScriptExecutedSuccessfully(false),
       mFetchHandlerWasAdded(false),
       mMainThreadObjectsForgotten(false),
       mIsChromeWorker(aIsChromeWorker),
       mParentFrozen(false),
       mIsSecureContext(
           IsNewWorkerSecureContext(mParent, mWorkerType, mLoadInfo)),
@@ -3155,17 +3155,17 @@ bool WorkerPrivate::IsOnCurrentThread() 
 void WorkerPrivate::ScheduleDeletion(WorkerRanOrNot aRanOrNot) {
   {
     // mWorkerThreadAccessible's accessor must be destructed before
     // the scheduled Runnable gets to run.
     MOZ_ACCESS_THREAD_BOUND(mWorkerThreadAccessible, data);
     MOZ_ASSERT(data->mChildWorkers.IsEmpty());
   }
   MOZ_ASSERT(mSyncLoopStack.IsEmpty());
-  MOZ_ASSERT(!mPendingEventQueueClearing);
+  MOZ_ASSERT(mPostSyncLoopOperations == 0);
 
   ClearMainEventQueue(aRanOrNot);
 #ifdef DEBUG
   if (WorkerRan == aRanOrNot) {
     nsIThread* currentThread = NS_GetCurrentThread();
     MOZ_ASSERT(currentThread);
     MOZ_ASSERT(!NS_HasPendingEvents(currentThread));
   }
@@ -3674,24 +3674,47 @@ bool WorkerPrivate::DestroySyncLoop(uint
     // This will delete |loopInfo|!
     mSyncLoopStack.RemoveElementAt(aLoopIndex);
   }
 
   auto queue =
       static_cast<ThreadEventQueue<EventQueue>*>(mThread->EventQueue());
   queue->PopEventQueue(nestedEventTarget);
 
-  if (mSyncLoopStack.IsEmpty() && mPendingEventQueueClearing) {
-    mPendingEventQueueClearing = false;
-    ClearMainEventQueue(WorkerRan);
+  if (mSyncLoopStack.IsEmpty()) {
+    if ((mPostSyncLoopOperations & ePendingEventQueueClearing)) {
+      ClearMainEventQueue(WorkerRan);
+    }
+
+    if ((mPostSyncLoopOperations & eDispatchCancelingRunnable)) {
+      DispatchCancelingRunnable();
+    }
+
+    mPostSyncLoopOperations = 0;
   }
 
   return result;
 }
 
+void WorkerPrivate::DispatchCancelingRunnable() {
+  // Here we use a normal runnable to know when the current JS chunk of code
+  // is finished. We cannot use a WorkerRunnable because they are not
+  // accepted any more by the worker, and we do not want to use a
+  // WorkerControlRunnable because they are immediately executed.
+  RefPtr<CancelingRunnable> r = new CancelingRunnable();
+  mThread->nsThread::Dispatch(r.forget(), NS_DISPATCH_NORMAL);
+
+  // At the same time, we want to be sure that we interrupt infinite loops.
+  // The following runnable starts a timer that cancel the worker, from the
+  // parent thread, after CANCELING_TIMEOUT millseconds.
+  RefPtr<CancelingWithTimeoutOnParentRunnable> rr =
+      new CancelingWithTimeoutOnParentRunnable(this);
+  rr->Dispatch();
+}
+
 void WorkerPrivate::StopSyncLoop(nsIEventTarget* aSyncLoopTarget,
                                  bool aResult) {
   AssertIsOnWorkerThread();
   AssertValidSyncLoop(aSyncLoopTarget);
 
   MOZ_ASSERT(!mSyncLoopStack.IsEmpty());
 
   for (uint32_t index = mSyncLoopStack.Length(); index > 0; index--) {
@@ -3966,45 +3989,35 @@ bool WorkerPrivate::NotifyInternal(Worke
   }
 
   // If this is the first time our status has changed then we need to clear the
   // main event queue.
   if (previousStatus == Running) {
     // 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.IsEmpty()) {
-      mPendingEventQueueClearing = true;
+      mPostSyncLoopOperations |= ePendingEventQueueClearing;
     } else {
       ClearMainEventQueue(WorkerRan);
     }
   }
 
   // If the worker script never ran, or failed to compile, we don't need to do
   // anything else.
   if (!GlobalScope()) {
     return true;
   }
 
   // Don't abort the script now, but we dispatch a runnable to do it when the
   // current JS frame is executed.
   if (aStatus == Closing) {
-    if (mSyncLoopStack.IsEmpty()) {
-      // Here we use a normal runnable to know when the current JS chunk of code
-      // is finished. We cannot use a WorkerRunnable because they are not
-      // accepted any more by the worker, and we do not want to use a
-      // WorkerControlRunnable because they are immediately executed.
-      RefPtr<CancelingRunnable> r = new CancelingRunnable();
-      mThread->nsThread::Dispatch(r.forget(), NS_DISPATCH_NORMAL);
-
-      // At the same time, we want to be sure that we interrupt infinite loops.
-      // The following runnable starts a timer that cancel the worker, from the
-      // parent thread, after CANCELING_TIMEOUT millseconds.
-      RefPtr<CancelingWithTimeoutOnParentRunnable> rr =
-          new CancelingWithTimeoutOnParentRunnable(this);
-      rr->Dispatch();
+    if (!mSyncLoopStack.IsEmpty()) {
+      mPostSyncLoopOperations |= eDispatchCancelingRunnable;
+    } else {
+      DispatchCancelingRunnable();
     }
     return true;
   }
 
   MOZ_ASSERT(aStatus == Canceling || aStatus == Killing);
 
   // Always abort the script.
   return false;
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -920,16 +920,23 @@ class WorkerPrivate : public RelativeTim
   }
 
   // Internal logic to dispatch a runnable. This is separate from Dispatch()
   // to allow runnables to be atomically dispatched in bulk.
   nsresult DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
                             nsIEventTarget* aSyncLoopTarget,
                             const MutexAutoLock& aProofOfLock);
 
+  // This method dispatches a simple runnable that starts the shutdown procedure
+  // after a self.close(). This method is called after a ClearMainEventQueue()
+  // to be sure that the canceling runnable is the only one in the queue.  We
+  // need this async operation to be sure that all the current JS code is
+  // executed.
+  void DispatchCancelingRunnable();
+
   class EventTarget;
   friend class EventTarget;
   friend class mozilla::dom::WorkerHolder;
   friend class AutoSyncLoopHolder;
 
   struct TimeoutInfo;
 
   class MemoryReporter;
@@ -1068,19 +1075,26 @@ class WorkerPrivate : public RelativeTim
     bool mTimerRunning;
     bool mRunningExpiredTimeouts;
     bool mPeriodicGCTimerRunning;
     bool mIdleGCTimerRunning;
     bool mOnLine;
   };
   ThreadBound<WorkerThreadAccessible> mWorkerThreadAccessible;
 
+  uint32_t mPostSyncLoopOperations;
+
+  // List of operations to do at the end of the last sync event loop.
+  enum {
+    ePendingEventQueueClearing = 0x01,
+    eDispatchCancelingRunnable = 0x02,
+  };
+
   bool mParentWindowPaused;
 
-  bool mPendingEventQueueClearing;
   bool mCancelAllPendingRunnables;
   bool mWorkerScriptExecutedSuccessfully;
   bool mFetchHandlerWasAdded;
   bool mMainThreadObjectsForgotten;
   bool mIsChromeWorker;
   bool mParentFrozen;
 
   // mIsSecureContext is set once in our constructor; after that it can be read