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 179646 d947c69441bcd987e482be609ab56e60ff9e043b
parent 179645 af1479b9a7ef9ab55803ed1545a1ac7a3776f342
child 179647 0ce418853a12b5e7c91ec1b66e24cf97665621cd
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersbsmedberg, mayhemer
bugs988881
milestone31.0a1
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