Bug 1339677. Part 4 - check if mMutex.mLock is tampered. draft
authorJW Wang <jwwang@mozilla.com>
Wed, 15 Feb 2017 16:42:26 +0800
changeset 485131 881e333ba7684b2a7b15417de0613578cc3c1b3f
parent 485130 a387cd4c1b83681a5cf123364e1605b8171f9018
child 485155 19357744678b1f6517fe97bfc97aa9e6f09e1814
push id45641
push userjwwang@mozilla.com
push dateThu, 16 Feb 2017 06:22:39 +0000
bugs1339677
milestone54.0a1
Bug 1339677. Part 4 - check if mMutex.mLock is tampered. MozReview-Commit-ID: H37EnCF0V4w
xpcom/threads/MozPromise.h
xpcom/threads/Mutex.h
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -183,16 +183,19 @@ public:
 protected:
   // MozPromise is the public type, and never constructed directly. Construct
   // a MozPromise::Private, defined below.
   MozPromise(const char* aCreationSite, bool aIsCompletionPromise)
     : mCreationSite(aCreationSite)
     , mMutex("MozPromise Mutex")
     , mHaveRequest(false)
     , mIsCompletionPromise(aIsCompletionPromise)
+#ifdef PROMISE_DEBUG
+    , mMagic4(mMutex.mLock)
+#endif
   {
     PROMISE_LOG("%s creating MozPromise (%p)", mCreationSite, this);
   }
 
 public:
   // MozPromise::Private allows us to separate the public interface (upon which
   // consumers of the promise may invoke methods like Then()) from the private
   // interface (upon which the creator of the promise may invoke Resolve() or
@@ -686,17 +689,17 @@ protected:
   private:
     Maybe<ResolveRejectFunction> mResolveRejectFunction; // Only accessed and deleted on dispatch thread.
   };
 
 public:
   void ThenInternal(AbstractThread* aResponseThread, ThenValueBase* aThenValue,
                     const char* aCallSite)
   {
-    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic);
+    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic && mMagic4 == mMutex.mLock);
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(aResponseThread->IsDispatchReliable());
     MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveRequest);
     mHaveRequest = true;
     PROMISE_LOG("%s invoking Then() [this=%p, aThenValue=%p, isPending=%d]",
                 aCallSite, this, aThenValue, (int) IsPending());
     if (!IsPending()) {
       aThenValue->Dispatch(this);
@@ -840,17 +843,17 @@ public:
   }
 
   // Note we expose the function AssertIsDead() instead of IsDead() since
   // checking IsDead() is a data race in the situation where the request is not
   // dead. Therefore we enforce the form |Assert(IsDead())| by exposing
   // AssertIsDead() only.
   void AssertIsDead()
   {
-    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic);
+    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic && mMagic4 == mMutex.mLock);
     MutexAutoLock lock(mMutex);
     for (auto&& then : mThenValues) {
       then->AssertIsDead();
     }
     for (auto&& chained : mChainedPromises) {
       chained->AssertIsDead();
     }
   }
@@ -899,16 +902,17 @@ protected:
       MOZ_ASSERT(!IsPending());
       MOZ_ASSERT(mThenValues.IsEmpty());
       MOZ_ASSERT(mChainedPromises.IsEmpty());
     }
 #ifdef PROMISE_DEBUG
     mMagic1 = 0;
     mMagic2 = 0;
     mMagic3 = 0;
+    mMagic4 = nullptr;
 #endif
   };
 
   const char* mCreationSite; // For logging
   Mutex mMutex;
   ResolveOrRejectValue mValue;
 #ifdef PROMISE_DEBUG
   uint32_t mMagic1 = sMagic;
@@ -918,52 +922,55 @@ protected:
   uint32_t mMagic2 = sMagic;
 #endif
   nsTArray<RefPtr<Private>> mChainedPromises;
 #ifdef PROMISE_DEBUG
   uint32_t mMagic3 = sMagic;
 #endif
   bool mHaveRequest;
   const bool mIsCompletionPromise;
+#ifdef PROMISE_DEBUG
+  void* mMagic4;
+#endif
 };
 
 template<typename ResolveValueT, typename RejectValueT, bool IsExclusive>
 class MozPromise<ResolveValueT, RejectValueT, IsExclusive>::Private
   : public MozPromise<ResolveValueT, RejectValueT, IsExclusive>
 {
 public:
   explicit Private(const char* aCreationSite, bool aIsCompletionPromise = false)
     : MozPromise(aCreationSite, aIsCompletionPromise) {}
 
   template<typename ResolveValueT_>
   void Resolve(ResolveValueT_&& aResolveValue, const char* aResolveSite)
   {
-    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic);
+    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic && mMagic4 == mMutex.mLock);
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(IsPending());
     PROMISE_LOG("%s resolving MozPromise (%p created at %s)", aResolveSite, this, mCreationSite);
     mValue.SetResolve(Forward<ResolveValueT_>(aResolveValue));
     DispatchAll();
   }
 
   template<typename RejectValueT_>
   void Reject(RejectValueT_&& aRejectValue, const char* aRejectSite)
   {
-    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic);
+    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic && mMagic4 == mMutex.mLock);
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(IsPending());
     PROMISE_LOG("%s rejecting MozPromise (%p created at %s)", aRejectSite, this, mCreationSite);
     mValue.SetReject(Forward<RejectValueT_>(aRejectValue));
     DispatchAll();
   }
 
   template<typename ResolveOrRejectValue_>
   void ResolveOrReject(ResolveOrRejectValue_&& aValue, const char* aSite)
   {
-    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic);
+    PROMISE_ASSERT(mMagic1 == sMagic && mMagic2 == sMagic && mMagic3 == sMagic && mMagic4 == mMutex.mLock);
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(IsPending());
     PROMISE_LOG("%s resolveOrRejecting MozPromise (%p created at %s)", aSite, this, mCreationSite);
     mValue = Forward<ResolveOrRejectValue_>(aValue);
     DispatchAll();
   }
 };
 
--- a/xpcom/threads/Mutex.h
+++ b/xpcom/threads/Mutex.h
@@ -105,16 +105,20 @@ public:
 private:
   OffTheBooksMutex();
   OffTheBooksMutex(const OffTheBooksMutex&);
   OffTheBooksMutex& operator=(const OffTheBooksMutex&);
 
   PRLock* mLock;
 
   friend class CondVar;
+
+  // MozPromise needs to access mLock for debugging purpose.
+  template<typename, typename, bool>
+  friend class MozPromise;
 };
 
 /**
  * Mutex
  * When possible, use MutexAutoLock/MutexAutoUnlock to lock/unlock this
  * mutex within a scope, instead of calling Lock/Unlock directly.
  */
 class Mutex : public OffTheBooksMutex