Bug 1250829 - add customized assertions for completion promises to facilitate promise chaining. r=bholley
MozReview-Commit-ID: L1TMiqQYexw
--- a/dom/media/gtest/TestMozPromise.cpp
+++ b/dom/media/gtest/TestMozPromise.cpp
@@ -215,9 +215,43 @@ 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__);
+ const size_t kLoopIteratos = 100;
+ for (size_t i = 0; i < kLoopIteratos; ++i) {
+ p = p->Then(queue, __func__,
+ [] (int aVal) {
+ EXPECT_EQ(aVal, 42);
+ },
+ [] () {}
+ )->CompletionPromise();
+
+ if (i == kLoopIteratos / 2) {
+ p->Then(queue, __func__,
+ [queue, &holder] () {
+ holder.Disconnect();
+ queue->BeginShutdown();
+ },
+ DO_FAIL);
+ }
+ }
+ // 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,38 @@ 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 want to assert that this ThenValues is dead - that is to say, that
+ // there are no consumers waiting for the result. In the case of a normal
+ // ThenValue, we check that it has been disconnected, which is the way
+ // that the consumer signals that it no longer wishes to hear about the
+ // result. If this ThenValue has a completion promise (which is mutually
+ // exclusive with being disconnectable), we recursively assert that every
+ // ThenValue associated with the completion promise is dead.
+ 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 +630,31 @@ public:
aCallSite, this, chainedPromise.get(), (int) IsPending());
if (!IsPending()) {
ForwardTo(chainedPromise);
} else {
mChainedPromises.AppendElement(chainedPromise);
}
}
+ // 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()
+ {
+ 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 +682,42 @@ 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());
+ AssertIsDead();
+ // 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));