Bug 1250829 - add customized assertions for completion promises to facilitate promise chaining. r=bholley draft
authorJW Wang <jwwang@mozilla.com>
Fri, 04 Mar 2016 16:57:48 +0800
changeset 336867 a5311503295b1ee6b200aeadd1a5807dabef5df3
parent 336866 dad104d732f2c3ba2e1fbc57248b4d62c6f7a1ff
child 336868 2945c6f951f3e42e65f602c3f18f4bf8324e0482
push id12200
push userjwwang@mozilla.com
push dateFri, 04 Mar 2016 08:58:14 +0000
reviewersbholley
bugs1250829
milestone47.0a1
Bug 1250829 - add customized assertions for completion promises to facilitate promise chaining. r=bholley MozReview-Commit-ID: 5sDpUJJAOyZ
dom/media/gtest/TestMozPromise.cpp
xpcom/threads/MozPromise.h
--- a/dom/media/gtest/TestMozPromise.cpp
+++ b/dom/media/gtest/TestMozPromise.cpp
@@ -215,9 +215,46 @@ TEST(MozPromise, PromiseAllReject)
       [queue] (float aRejectValue) -> void {
         EXPECT_EQ(aRejectValue, 32.0);
         queue->BeginShutdown();
       }
     );
   });
 }
 
+// Test we don't hit the assertions in MozPromise when exercising promise
+// chaining upon task queue shutdown.
+TEST(MozPromise, Chaining)
+{
+  AutoTaskQueue atq;
+  RefPtr<TaskQueue> queue = atq.Queue();
+  MozPromiseRequestHolder<TestPromise> holder;
+
+  RunOnTaskQueue(queue, [queue, &holder] () {
+    auto p = TestPromise::CreateAndResolve(42, __func__);
+    int loop = 100;
+    for (int i = 0; i < loop; ++i) {
+      p = p->Then(queue, __func__,
+        [] (int aVal) {
+          EXPECT_EQ(aVal, 42);
+        },
+        [] () {}
+      )->CompletionPromise();
+
+      if (i == loop / 2) {
+        p->Then(queue, __func__,
+          [queue, &holder] () {
+            holder.Disconnect();
+            queue->BeginShutdown();
+          },
+          [queue, &holder] () {
+            holder.Disconnect();
+            queue->BeginShutdown();
+          });
+      }
+    }
+    // We will hit the assertion if we don't disconnect the leaf Request
+    // in the promise chain.
+    holder.Begin(p->Then(queue, __func__, [] () {}, [] () {}));
+  });
+}
+
 #undef DO_FAIL
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -160,20 +160,21 @@ public:
   private:
     Maybe<ResolveValueType> mResolveValue;
     Maybe<RejectValueType> mRejectValue;
   };
 
 protected:
   // MozPromise is the public type, and never constructed directly. Construct
   // a MozPromise::Private, defined below.
-  explicit MozPromise(const char* aCreationSite)
+  MozPromise(const char* aCreationSite, bool aIsCompletionPromise)
     : mCreationSite(aCreationSite)
     , mMutex("MozPromise Mutex")
     , mHaveRequest(false)
+    , mIsCompletionPromise(aIsCompletionPromise)
   {
     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
@@ -273,16 +274,18 @@ public:
     virtual void Disconnect() = 0;
 
     // MSVC complains when an inner class (ThenValueBase::{Resolve,Reject}Runnable)
     // tries to access an inherited protected member.
     bool IsDisconnected() const { return mDisconnected; }
 
     virtual MozPromise* CompletionPromise() = 0;
 
+    virtual void AssertIsDead() = 0;
+
   protected:
     Request() : mComplete(false), mDisconnected(false) {}
     virtual ~Request() {}
 
     bool mComplete;
     bool mDisconnected;
   };
 
@@ -304,17 +307,19 @@ protected:
         : mThenValue(aThenValue)
         , mPromise(aPromise)
       {
         MOZ_DIAGNOSTIC_ASSERT(!mPromise->IsPending());
       }
 
       ~ResolveOrRejectRunnable()
       {
-        MOZ_DIAGNOSTIC_ASSERT(!mThenValue || mThenValue->IsDisconnected());
+        if (mThenValue) {
+          mThenValue->AssertIsDead();
+        }
       }
 
       NS_IMETHODIMP Run()
       {
         PROMISE_LOG("ResolveOrRejectRunnable::Run() [this=%p]", this);
         mThenValue->DoResolveOrReject(mPromise->Value());
         mThenValue = nullptr;
         mPromise = nullptr;
@@ -329,21 +334,33 @@ protected:
     explicit ThenValueBase(AbstractThread* aResponseTarget, const char* aCallSite)
       : mResponseTarget(aResponseTarget), mCallSite(aCallSite) {}
 
     MozPromise* CompletionPromise() override
     {
       MOZ_DIAGNOSTIC_ASSERT(mResponseTarget->IsCurrentThreadIn());
       MOZ_DIAGNOSTIC_ASSERT(!Request::mComplete);
       if (!mCompletionPromise) {
-        mCompletionPromise = new MozPromise::Private("<completion promise>");
+        mCompletionPromise = new MozPromise::Private(
+          "<completion promise>", true /* aIsCompletionPromise */);
       }
       return mCompletionPromise;
     }
 
+    void AssertIsDead() override
+    {
+      // We can't assert IsDisconnected() when a completion promise is present
+      // since we don't support it. See comments in Disconnect() below.
+      if (mCompletionPromise) {
+        mCompletionPromise->AssertIsDead();
+      } else {
+        MOZ_DIAGNOSTIC_ASSERT(Request::mDisconnected);
+      }
+    }
+
     void Dispatch(MozPromise *aPromise)
     {
       aPromise->mMutex.AssertCurrentThreadOwns();
       MOZ_ASSERT(!aPromise->IsPending());
 
       RefPtr<nsRunnable> runnable =
         static_cast<nsRunnable*>(new (typename ThenValueBase::ResolveOrRejectRunnable)(this, aPromise));
       PROMISE_LOG("%s Then() call made from %s [Runnable=%p, Promise=%p, ThenValue=%p]",
@@ -608,16 +625,27 @@ public:
                 aCallSite, this, chainedPromise.get(), (int) IsPending());
     if (!IsPending()) {
       ForwardTo(chainedPromise);
     } else {
       mChainedPromises.AppendElement(chainedPromise);
     }
   }
 
+  void AssertIsDead()
+  {
+    MutexAutoLock lock(mMutex);
+    for (auto&& then : mThenValues) {
+      then->AssertIsDead();
+    }
+    for (auto&& chained : mChainedPromises) {
+      chained->AssertIsDead();
+    }
+  }
+
 protected:
   bool IsPending() const { return mValue.IsNothing(); }
   const ResolveOrRejectValue& Value() const
   {
     // This method should only be called once the value has stabilized. As
     // such, we don't need to acquire the lock here.
     MOZ_DIAGNOSTIC_ASSERT(!IsPending());
     return mValue;
@@ -645,35 +673,41 @@ protected:
     } else {
       aOther->Reject(mValue.RejectValue(), "<chained promise>");
     }
   }
 
   virtual ~MozPromise()
   {
     PROMISE_LOG("MozPromise::~MozPromise [this=%p]", this);
-    MOZ_ASSERT(!IsPending());
-    MOZ_ASSERT(mThenValues.IsEmpty());
-    MOZ_ASSERT(mChainedPromises.IsEmpty());
+    // We can't guarantee a completion promise will always be revolved or
+    // rejected since ResolveOrRejectRunnable might not run when dispatch fails.
+    if (!mIsCompletionPromise) {
+      MOZ_ASSERT(!IsPending());
+      MOZ_ASSERT(mThenValues.IsEmpty());
+      MOZ_ASSERT(mChainedPromises.IsEmpty());
+    }
   };
 
   const char* mCreationSite; // For logging
   Mutex mMutex;
   ResolveOrRejectValue mValue;
   nsTArray<RefPtr<ThenValueBase>> mThenValues;
   nsTArray<RefPtr<Private>> mChainedPromises;
   bool mHaveRequest;
+  const bool mIsCompletionPromise;
 };
 
 template<typename ResolveValueT, typename RejectValueT, bool IsExclusive>
 class MozPromise<ResolveValueT, RejectValueT, IsExclusive>::Private
   : public MozPromise<ResolveValueT, RejectValueT, IsExclusive>
 {
 public:
-  explicit Private(const char* aCreationSite) : MozPromise(aCreationSite) {}
+  explicit Private(const char* aCreationSite, bool aIsCompletionPromise = false)
+    : MozPromise(aCreationSite, aIsCompletionPromise) {}
 
   template<typename ResolveValueT_>
   void Resolve(ResolveValueT_&& aResolveValue, const char* aResolveSite)
   {
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(IsPending());
     PROMISE_LOG("%s resolving MozPromise (%p created at %s)", aResolveSite, this, mCreationSite);
     mValue.SetResolve(Forward<ResolveValueT_>(aResolveValue));