Bug 1396110 - Fix races during watchdog shutdown. r=billm, a=sledru
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 20 Sep 2017 15:23:28 -0700
changeset 432011 c048148b2f2d6ea31c20d9068947bbb7945348bf
parent 432010 bd13ea56e690c84920e5f548e0a171e0966c152f
child 432012 dd4bcf227b95d5a8767b3873f85b16c4c7eafd09
push id7870
push userryanvm@gmail.com
push dateMon, 02 Oct 2017 13:47:52 +0000
treeherdermozilla-beta@b7b894f5fad8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, sledru
bugs1396110
milestone57.0
Bug 1396110 - Fix races during watchdog shutdown. r=billm, a=sledru This patch also ensures that we won't accidentally try stopping slow scripts when we're shutting down the watchdog manager. MozReview-Commit-ID: EMb0enfivd8
js/xpconnect/src/XPCJSContext.cpp
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -293,16 +293,24 @@ class WatchdogManager : public nsIObserv
     void
     UnregisterContext(XPCJSContext* aContext)
     {
         MOZ_ASSERT(NS_IsMainThread());
         AutoLockWatchdog lock(mWatchdog);
 
         // aContext must be in one of our two lists, simply remove it.
         aContext->LinkedListElement<XPCJSContext>::remove();
+
+#ifdef DEBUG
+        // If this was the last context, we should have already shut down
+        // the watchdog.
+        if (mActiveContexts.isEmpty() && mInactiveContexts.isEmpty()) {
+            MOZ_ASSERT(!mWatchdog);
+        }
+#endif
     }
 
     // Context statistics. These live on the watchdog manager, are written
     // from the main thread, and are read from the watchdog thread (holding
     // the lock in each case).
     void RecordContextActivity(XPCJSContext* aContext, bool active)
     {
         // The watchdog reads this state, so acquire the lock before writing it.
@@ -468,17 +476,16 @@ WatchdogMain(void* arg)
 
     Watchdog* self = static_cast<Watchdog*>(arg);
     WatchdogManager* manager = self->Manager();
 
     // Lock lasts until we return
     AutoLockWatchdog lock(self);
 
     MOZ_ASSERT(self->Initialized());
-    MOZ_ASSERT(!self->ShuttingDown());
     while (!self->ShuttingDown()) {
         // Sleep only 1 second if recently (or currently) active; otherwise, hibernate
         if (manager->IsAnyContextActive() ||
             manager->TimeSinceLastActiveContext() <= PRTime(2*PR_USEC_PER_SEC))
         {
             self->Sleep(PR_TicksPerSecond());
         } else {
             manager->RecordTimestamp(TimestampWatchdogHibernateStart);
@@ -498,17 +505,17 @@ WatchdogMain(void* arg)
         // invoke the interrupt callback after only half the timeout has
         // elapsed. The callback simply records the fact that it was called in
         // the mSlowScriptSecondHalf flag. Then we wait another (timeout/2)
         // seconds and invoke the callback again. This time around it sees
         // mSlowScriptSecondHalf is set and so it shows the slow script
         // dialog. If the computer is put to sleep during one of the (timeout/2)
         // periods, the script still has the other (timeout/2) seconds to
         // finish.
-        if (manager->IsAnyContextActive()) {
+        if (!self->ShuttingDown() && manager->IsAnyContextActive()) {
             bool debuggerAttached = false;
             nsCOMPtr<nsIDebug2> dbg = do_GetService("@mozilla.org/xpcom/debug;1");
             if (dbg)
                 dbg->GetIsDebuggerAttached(&debuggerAttached);
             if (debuggerAttached) {
                 // We won't be interrupting these scripts anyway.
                 continue;
             }
@@ -870,23 +877,27 @@ XPCJSContext::~XPCJSContext()
 
     js::SetActivityCallback(Context(), nullptr, nullptr);
 
     // Clear any pending exception.  It might be an XPCWrappedJS, and if we try
     // to destroy it later we will crash.
     SetPendingException(nullptr);
 
     // If we're the last XPCJSContext around, clean up the watchdog manager.
-    mWatchdogManager->UnregisterContext(this);
     if (--sInstanceCount == 0) {
         if (mWatchdogManager->GetWatchdog()) {
             mWatchdogManager->StopWatchdog();
         }
+
+        mWatchdogManager->UnregisterContext(this);
         mWatchdogManager->Shutdown();
         sWatchdogInstance = nullptr;
+    } else {
+        // Otherwise, simply remove ourselves from the list.
+        mWatchdogManager->UnregisterContext(this);
     }
 
     if (mCallContext)
         mCallContext->SystemIsBeingShutDown();
 
     auto rtPrivate = static_cast<PerThreadAtomCache*>(JS_GetContextPrivate(Context()));
     delete rtPrivate;
     JS_SetContextPrivate(Context(), nullptr);