Bug 988881: clean up CryptoTask (SignedJar) tasks instead of leaking them r=bsmedberg,mayhemer
authorRandell Jesup <rjesup@jesup.org>
Thu, 17 Apr 2014 02:18:04 -0400
changeset 197794 d947c69441bcd987e482be609ab56e60ff9e043b
parent 197793 af1479b9a7ef9ab55803ed1545a1ac7a3776f342
child 197795 0ce418853a12b5e7c91ec1b66e24cf97665621cd
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg, mayhemer
bugs988881
milestone31.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 988881: clean up CryptoTask (SignedJar) tasks instead of leaking them r=bsmedberg,mayhemer
security/manager/ssl/src/CryptoTask.cpp
security/manager/ssl/src/CryptoTask.h
--- a/security/manager/ssl/src/CryptoTask.cpp
+++ b/security/manager/ssl/src/CryptoTask.cpp
@@ -13,27 +13,16 @@ CryptoTask::~CryptoTask()
   MOZ_ASSERT(mReleasedNSSResources);
 
   nsNSSShutDownPreventionLock lock;
   if (!isAlreadyShutDown()) {
     shutdown(calledFromObject);
   }
 }
 
-nsresult
-CryptoTask::Dispatch(const nsACString & taskThreadName)
-{
-  nsCOMPtr<nsIThread> thread;
-  nsresult rv = NS_NewThread(getter_AddRefs(thread), this);
-  if (thread) {
-    NS_SetThreadName(thread, taskThreadName);
-  }
-  return rv;
-}
-
 NS_IMETHODIMP
 CryptoTask::Run()
 {
   if (!NS_IsMainThread()) {
     nsNSSShutDownPreventionLock locker;
     if (isAlreadyShutDown()) {
       mRv = NS_ERROR_NOT_AVAILABLE;
     } else {
@@ -47,16 +36,23 @@ CryptoTask::Run()
     // CryptoTasks have consistent behavior regardless of whether NSS is shut
     // down between CalculateResult being called and CallCallback being called.
     if (!mReleasedNSSResources) {
       mReleasedNSSResources = true;
       ReleaseNSSResources();
     }
 
     CallCallback(mRv);
+
+    // Not all uses of CryptoTask use a transient thread
+    if (mThread) {
+      // Don't leak threads!
+      mThread->Shutdown(); // can't Shutdown from the thread itself, darn
+      mThread = nullptr;
+    }
   }
 
   return NS_OK;
 }
 
 void
 CryptoTask::virtualDestroyNSSReference()
 {
--- a/security/manager/ssl/src/CryptoTask.h
+++ b/security/manager/ssl/src/CryptoTask.h
@@ -34,17 +34,22 @@ class CryptoTask : public nsRunnable,
                    public nsNSSShutDownObject
 {
 public:
   template <size_t LEN>
   nsresult Dispatch(const char (&taskThreadName)[LEN])
   {
     static_assert(LEN <= 15,
                   "Thread name must be no more than 15 characters");
-    return Dispatch(nsDependentCString(taskThreadName));
+    // Can't add 'this' as the event to run, since mThread may not be set yet
+    nsresult rv = NS_NewNamedThread(taskThreadName, getter_AddRefs(mThread));
+    if (NS_SUCCEEDED(rv)) {
+      rv = mThread->Dispatch(this, NS_DISPATCH_NORMAL);
+    }
+    return rv;
   }
 
 protected:
   CryptoTask()
     : mRv(NS_ERROR_NOT_INITIALIZED),
       mReleasedNSSResources(false)
   {
   }
@@ -71,17 +76,17 @@ protected:
    * be called.
    */
   virtual void CallCallback(nsresult rv) = 0;
 
 private:
   NS_IMETHOD Run() MOZ_OVERRIDE MOZ_FINAL;
   virtual void virtualDestroyNSSReference() MOZ_OVERRIDE MOZ_FINAL;
 
-  nsresult Dispatch(const nsACString & taskThreadName);
-
   nsresult mRv;
   bool mReleasedNSSResources;
+
+  nsCOMPtr<nsIThread> mThread;
 };
 
 } // namespace mozilla
 
 #endif // mozilla__CryptoTask_h