Bug 1323155 - fix data race in mCompletionPromise. r=gerald draft
authorJW Wang <jwwang@mozilla.com>
Tue, 13 Dec 2016 16:55:14 +0800
changeset 449353 4f2222dbf122840c4b250bc2fc1feef948332cd4
parent 449352 e42afa173c484be4025a5cef561d7f7f5079b4d4
child 449354 e51cf7cd2615a3d708874239cfc6ad7e8db67ced
push id38549
push userbmo:jyavenard@mozilla.com
push dateWed, 14 Dec 2016 01:34:13 +0000
reviewersgerald
bugs1323155
milestone53.0a1
Bug 1323155 - fix data race in mCompletionPromise. r=gerald MozReview-Commit-ID: J2TVNWvx9pb
xpcom/threads/MozPromise.h
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -623,35 +623,41 @@ public:
   MOZ_MUST_USE RefPtr<MozPromise>
   ThenPromise(AbstractThread* aResponseThread, const char* aCallSite, ThisType* aThisVal,
               ResolveMethodType aResolveMethod, RejectMethodType aRejectMethod)
   {
     using ThenType = MethodThenValue<ThisType, ResolveMethodType, RejectMethodType>;
     RefPtr<ThenValueBase> thenValue = new ThenType(
       aResponseThread, aThisVal, aResolveMethod, aRejectMethod, aCallSite);
     // mCompletionPromise must be created before ThenInternal() to avoid race.
-    thenValue->mCompletionPromise = new MozPromise::Private(
+    RefPtr<MozPromise> p = new MozPromise::Private(
       "<completion promise>", true /* aIsCompletionPromise */);
+    thenValue->mCompletionPromise = p;
+    // Note ThenInternal() might nullify mCompletionPromise before return.
+    // So we need to return p instead of mCompletionPromise.
     ThenInternal(aResponseThread, thenValue, aCallSite);
-    return thenValue->mCompletionPromise;
+    return p;
   }
 
   template<typename ResolveFunction, typename RejectFunction>
   MOZ_MUST_USE RefPtr<MozPromise>
   ThenPromise(AbstractThread* aResponseThread, const char* aCallSite,
               ResolveFunction&& aResolveFunction, RejectFunction&& aRejectFunction)
   {
     using ThenType = FunctionThenValue<ResolveFunction, RejectFunction>;
     RefPtr<ThenValueBase> thenValue = new ThenType(
       aResponseThread, Move(aResolveFunction), Move(aRejectFunction), aCallSite);
     // mCompletionPromise must be created before ThenInternal() to avoid race.
-    thenValue->mCompletionPromise = new MozPromise::Private(
+    RefPtr<MozPromise> p = new MozPromise::Private(
       "<completion promise>", true /* aIsCompletionPromise */);
+    thenValue->mCompletionPromise = p;
+    // Note ThenInternal() might nullify mCompletionPromise before return.
+    // So we need to return p instead of mCompletionPromise.
     ThenInternal(aResponseThread, thenValue, aCallSite);
-    return thenValue->mCompletionPromise;
+    return p;
   }
 
   void ChainTo(already_AddRefed<Private> aChainedPromise, const char* aCallSite)
   {
     MutexAutoLock lock(mMutex);
     MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveRequest);
     mHaveRequest = true;
     RefPtr<Private> chainedPromise = aChainedPromise;