Bug 996487: don't null out mThread while committing thread suicide r=bsmedberg
authorRandell Jesup <rjesup@jesup.org>
Tue, 22 Apr 2014 15:32:13 -0400
changeset 179964 d4df499022c99c391dc2d1c2a0289d492357170e
parent 179963 5ea518e77780c438e518f01c1062c4b4c4f79caf
child 179965 2e2566a604f18eb8ab3a6d6d929c6af2bfffc3df
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersbsmedberg
bugs996487
milestone31.0a1
Bug 996487: don't null out mThread while committing thread suicide r=bsmedberg
content/media/AudioStream.cpp
content/media/AudioStream.h
security/manager/ssl/src/CryptoTask.cpp
security/manager/ssl/src/CryptoTask.h
--- a/content/media/AudioStream.cpp
+++ b/content/media/AudioStream.cpp
@@ -516,17 +516,21 @@ AudioStream::CheckForStart()
 }
 
 NS_IMETHODIMP
 AudioInitTask::Run()
 {
   MOZ_ASSERT(mThread);
   if (NS_IsMainThread()) {
     mThread->Shutdown(); // can't Shutdown from the thread itself, darn
-    mThread = nullptr;
+    // 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;
   }
 
   nsresult rv = mAudioStream->OpenCubeb(mParams, mLatencyRequest);
 
   // and now kill this thread
   NS_DispatchToMainThread(this);
   return rv;
--- a/content/media/AudioStream.h
+++ b/content/media/AudioStream.h
@@ -440,16 +440,17 @@ public:
     , mParams(aParams)
   {}
 
   nsresult Dispatch()
   {
     // Can't add 'this' as the event to run, since mThread may not be set yet
     nsresult rv = NS_NewNamedThread("CubebInit", getter_AddRefs(mThread));
     if (NS_SUCCEEDED(rv)) {
+      // Note: event must not null out mThread!
       rv = mThread->Dispatch(this, NS_DISPATCH_NORMAL);
     }
     return rv;
   }
 
 protected:
   virtual ~AudioInitTask() {};
 
--- a/security/manager/ssl/src/CryptoTask.cpp
+++ b/security/manager/ssl/src/CryptoTask.cpp
@@ -41,17 +41,21 @@ CryptoTask::Run()
     }
 
     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;
+      // 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;
 }
 
 void
 CryptoTask::virtualDestroyNSSReference()
--- a/security/manager/ssl/src/CryptoTask.h
+++ b/security/manager/ssl/src/CryptoTask.h
@@ -37,16 +37,17 @@ public:
   template <size_t LEN>
   nsresult Dispatch(const char (&taskThreadName)[LEN])
   {
     static_assert(LEN <= 15,
                   "Thread name must be no more than 15 characters");
     // 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)) {
+      // Note: event must not null out mThread!
       rv = mThread->Dispatch(this, NS_DISPATCH_NORMAL);
     }
     return rv;
   }
 
 protected:
   CryptoTask()
     : mRv(NS_ERROR_NOT_INITIALIZED),