bug 1439383 - clean up the load loadable roots thread when we're done with it r=froydnj,jcj
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 16 Mar 2018 16:50:19 -0700
changeset 409021 5be07e86738e8993022e1c507b5b09e602872a33
parent 409020 857451e02515b258368c5197c01aaa7b0ce63a8b
child 409022 e5031ab7e62849a9c050128a6ce39ea160a21a6b
push id33671
push usercsabou@mozilla.com
push dateTue, 20 Mar 2018 22:23:32 +0000
treeherdermozilla-central@e2e874ceae78 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, jcj
bugs1439383
milestone61.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 1439383 - clean up the load loadable roots thread when we're done with it r=froydnj,jcj MozReview-Commit-ID: J5GnpwxYguz
security/manager/ssl/nsNSSComponent.cpp
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -1010,16 +1010,33 @@ LoadLoadableRootsTask::Dispatch()
 
   // Note: event must not null out mThread!
   return mThread->Dispatch(this, NS_DISPATCH_NORMAL);
 }
 
 NS_IMETHODIMP
 LoadLoadableRootsTask::Run()
 {
+  // First we Run() on the "LoadRoots" thread, do our work, and then we Run()
+  // again on the main thread so we can shut down the thread (since we don't
+  // need it any more). We can't shut down the thread while we're *on* the
+  // thread, which is why we do the dispatch to the main thread. CryptoTask.cpp
+  // (which informs this code) says that we can't null out mThread. This appears
+  // to be because its refcount could be decreased while this dispatch is being
+  // processed, so it might get prematurely destroyed. I'm not sure this makes
+  // sense but it'll get cleaned up in our destructor anyway, so it's fine to
+  // not null it out here (as long as we don't run twice on the main thread,
+  // which shouldn't be possible).
+  if (NS_IsMainThread()) {
+    if (mThread) {
+      mThread->Shutdown();
+    }
+    return NS_OK;
+  }
+
   nsresult rv = LoadLoadableRoots();
   if (NS_FAILED(rv)) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("LoadLoadableRoots failed"));
     // We don't return rv here because then BlockUntilLoadableRootsLoaded will
     // just wait forever. Instead we'll save its value (below) so we can inform
     // code that relies on the roots module being present that loading it
     // failed.
   }
@@ -1037,17 +1054,19 @@ LoadLoadableRootsTask::Run()
     // Cache the result of LoadLoadableRoots so BlockUntilLoadableRootsLoaded
     // can return it to all callers later.
     mNSSComponent->mLoadableRootsLoadedResult = rv;
     rv = mNSSComponent->mLoadableRootsLoadedMonitor.NotifyAll();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
-  return NS_OK;
+
+  // Go back to the main thread to clean up this worker thread.
+  return NS_DispatchToMainThread(this);
 }
 
 nsresult
 nsNSSComponent::HasActiveSmartCards(bool& result)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_SAME_THREAD;