Bug 1022376: Properly shut down LoadMonitor threads r=jib
authorRandell Jesup <rjesup@jesup.org>
Sun, 08 Jun 2014 18:37:14 -0400
changeset 207821 b5067fbd502ee7ff5d7e6b0a54edcd6d0fcd847f
parent 207820 d01c93428c326891c908cdda4177406e55e69f31
child 207822 6cc9ac1f57fabc4466b5beb0889f731fa804e5b1
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1022376
milestone32.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 1022376: Properly shut down LoadMonitor threads r=jib
content/media/webrtc/LoadManager.cpp
content/media/webrtc/LoadMonitor.cpp
--- a/content/media/webrtc/LoadManager.cpp
+++ b/content/media/webrtc/LoadManager.cpp
@@ -53,16 +53,17 @@ LoadManager::LoadManager(int aLoadMeasur
   MOZ_ASSERT(mHighLoadThreshold > mLowLoadThreshold);
   mLoadMonitor = new LoadMonitor(mLoadMeasurementInterval);
   mLoadMonitor->Init(mLoadMonitor);
   mLoadMonitor->SetLoadChangeCallback(this);
 }
 
 LoadManager::~LoadManager()
 {
+  LOG(("LoadManager: shutting down LoadMonitor"));
   mLoadMonitor->Shutdown();
 }
 
 void
 LoadManager::LoadChanged(float aSystemLoad, float aProcesLoad)
 {
   // Update total load, and total amount of measured seconds.
   mLoadSum += aSystemLoad;
--- a/content/media/webrtc/LoadMonitor.cpp
+++ b/content/media/webrtc/LoadMonitor.cpp
@@ -144,21 +144,27 @@ public:
   }
 
 private:
   nsRefPtr<LoadMonitor> mLoadMonitor;
 };
 
 void LoadMonitor::Shutdown()
 {
-  MutexAutoLock lock(mLock);
   if (mLoadInfoThread) {
-    mShutdownPending = true;
-    mCondVar.Notify();
+    {
+      MutexAutoLock lock(mLock);
+      LOG(("LoadMonitor: shutting down"));
+      mShutdownPending = true;
+      mCondVar.Notify();
+    }
 
+    // Note: can't just call ->Shutdown() from here; that spins the event
+    // loop here, causing re-entrancy issues if we're invoked from cycle
+    // collection.  Argh.
     mLoadInfoThread = nullptr;
 
     nsRefPtr<LoadMonitorRemoveObserver> remObsRunner = new LoadMonitorRemoveObserver(this);
     if (!NS_IsMainThread()) {
       NS_DispatchToMainThread(remObsRunner);
     } else {
       remObsRunner->Run();
     }
@@ -511,26 +517,41 @@ nsresult LoadInfo::UpdateProcessLoad() {
 #endif
   return NS_OK;
 }
 
 class LoadInfoCollectRunner : public nsRunnable
 {
 public:
   LoadInfoCollectRunner(nsRefPtr<LoadMonitor> loadMonitor,
-                        RefPtr<LoadInfo> loadInfo)
-    : mLoadUpdateInterval(loadMonitor->mLoadUpdateInterval),
+                        RefPtr<LoadInfo> loadInfo,
+                        nsIThread *loadInfoThread)
+    : mThread(loadInfoThread),
+      mLoadUpdateInterval(loadMonitor->mLoadUpdateInterval),
       mLoadNoiseCounter(0)
   {
     mLoadMonitor = loadMonitor;
     mLoadInfo = loadInfo;
   }
 
   NS_IMETHOD Run()
   {
+    if (NS_IsMainThread()) {
+      if (mThread) {
+        // Don't leak threads!
+        mThread->Shutdown(); // can't Shutdown from the thread itself, darn
+        // Don't null out mThread!
+        // See bug 999104.  We must hold a ref to the thread across Dispatch()
+        // since the internal mThread ref could be released while processing
+        // the Dispatch(), and Dispatch/PutEvent itself doesn't hold a ref; it
+        // assumes the caller does.
+      }
+      return NS_OK;
+    }
+
     MutexAutoLock lock(mLoadMonitor->mLock);
     while (!mLoadMonitor->mShutdownPending) {
       mLoadInfo->UpdateSystemLoad();
       mLoadInfo->UpdateProcessLoad();
       float sysLoad = mLoadInfo->GetSystemLoad();
       float procLoad = mLoadInfo->GetProcessLoad();
 
       if ((++mLoadNoiseCounter % (LOG_MANY_ENABLED() ? 1 : 10)) == 0) {
@@ -538,20 +559,23 @@ public:
         mLoadNoiseCounter = 0;
       }
       mLoadMonitor->SetSystemLoad(sysLoad);
       mLoadMonitor->SetProcessLoad(procLoad);
       mLoadMonitor->FireCallbacks();
 
       mLoadMonitor->mCondVar.Wait(PR_MillisecondsToInterval(mLoadUpdateInterval));
     }
+    // ok, we need to exit safely and can't shut ourselves down (DARN)
+    NS_DispatchToMainThread(this);
     return NS_OK;
   }
 
 private:
+  nsCOMPtr<nsIThread> mThread;
   RefPtr<LoadInfo> mLoadInfo;
   nsRefPtr<LoadMonitor> mLoadMonitor;
   int mLoadUpdateInterval;
   int mLoadNoiseCounter;
 };
 
 void
 LoadMonitor::SetProcessLoad(float load) {
@@ -600,17 +624,17 @@ LoadMonitor::Init(nsRefPtr<LoadMonitor> 
   }
 
   nsRefPtr<LoadMonitorAddObserver> addObsRunner = new LoadMonitorAddObserver(self);
   NS_DispatchToMainThread(addObsRunner);
 
   NS_NewNamedThread("Sys Load Info", getter_AddRefs(mLoadInfoThread));
 
   nsRefPtr<LoadInfoCollectRunner> runner =
-    new LoadInfoCollectRunner(self, load_info);
+    new LoadInfoCollectRunner(self, load_info, mLoadInfoThread);
   mLoadInfoThread->Dispatch(runner, NS_DISPATCH_NORMAL);
 
   return NS_OK;
 }
 
 void
 LoadMonitor::SetLoadChangeCallback(LoadNotificationCallback* aCallback)
 {