Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-21 09:15:35 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1135424 - Allow MediaPromise dispatch to fail if the ThenValue has been disconnected. r=mattwoodrow

Browse Source 
The original idea behind the current model was that we wanted ironclad guarantees
that consumers would always get a callback on their promise. But we now have use
cases where the consumer wants to forget about a promise (using the new
Disconnect()) feature, and in some cases wants to shut down the task queue that
the response is going to be dispatched on. In the case of this bug, we want to
avoid waiting for the longest outstanding timer promise to be resolved before
shutting down the MDSM.

So this patch fixes up the pieces needed to make this work:
* Loosening our invariants to allow dispatch targets to be released on any thread,
  since MediaTaskQueue and nsIEventTarget both have thread-safe refcounting.
* Releasing mThisVal in Disconnect, so that we no longer depend on successful
  dispatch to release it on the correct (dispatch) thread.
* Fiddling with various assertions.

We also make some assertions fatal in nightly/aurora builds while we're at it.
This commit is contained in:
Bobby Holley 2015-03-11 11:29:50 -07:00
parent 23214cf141
commit 1d46643778
1 changed files with 41 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
dom/media/MediaPromise.h
View File

@ -16,6 +16,7 @@
#include "mozilla/Maybe.h"
#include "mozilla/Mutex.h"
#include "mozilla/Monitor.h"
#include "mozilla/unused.h"
/* Polyfill __func__ on MSVC for consumers to pass to the MediaPromise API. */
#ifdef _MSC_VER
@ -108,18 +109,11 @@ public:
public:
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Consumer)
void Disconnect()
{
AssertOnDispatchThread();
MOZ_DIAGNOSTIC_ASSERT(!mComplete);
mDisconnected = true;
}
virtual void Disconnect() = 0;
#ifdef DEBUG
virtual void AssertOnDispatchThread() = 0;
#else
void AssertOnDispatchThread() {}
#endif
// MSVC complains when an inner class (ThenValueBase::{Resolve,Reject}Runnable)
// tries to access an inherited protected member.
bool IsDisconnected() const { return mDisconnected; }
protected:
Consumer() : mComplete(false), mDisconnected(false) {}
@ -149,7 +143,7 @@ protected:
~ResolveRunnable()
{
MOZ_ASSERT(!mThenValue);
MOZ_DIAGNOSTIC_ASSERT(!mThenValue || mThenValue->IsDisconnected());
}
NS_IMETHODIMP Run()
@ -174,7 +168,7 @@ protected:
~RejectRunnable()
{
MOZ_ASSERT(!mThenValue);
MOZ_DIAGNOSTIC_ASSERT(!mThenValue || mThenValue->IsDisconnected());
}
NS_IMETHODIMP Run()
@ -252,35 +246,54 @@ protected:
PROMISE_LOG("%s Then() call made from %s [Runnable=%p, Promise=%p, ThenValue=%p]",
resolved ? "Resolving" : "Rejecting", ThenValueBase::mCallSite,
runnable.get(), aPromise, this);
DebugOnly<nsresult> rv = detail::DispatchMediaPromiseRunnable(mResponseTarget, runnable);
MOZ_ASSERT(NS_SUCCEEDED(rv));
nsresult rv = detail::DispatchMediaPromiseRunnable(mResponseTarget, runnable);
unused << rv;
// NB: mDisconnected is only supposed to be accessed on the dispatch
// thread. However, we require the consumer to have disconnected any
// oustanding promise requests _before_ initiating shutdown on the
// thread or task queue. So the only non-buggy scenario for dispatch
// failing involves the target thread being unable to manipulate the
// ThenValue (since it's been disconnected), so it's safe to read here.
MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv) || Consumer::mDisconnected);
}
#ifdef DEBUG
virtual void AssertOnDispatchThread() MOZ_OVERRIDE
void AssertOnDispatchThread()
{
detail::AssertOnThread(mResponseTarget);
}
#else
void AssertOnDispatchThread() {}
#endif
virtual void Disconnect() MOZ_OVERRIDE
{
AssertOnDispatchThread();
MOZ_DIAGNOSTIC_ASSERT(!Consumer::mComplete);
Consumer::mDisconnected = true;
// If a Consumer has been disconnected, we don't guarantee that the
// resolve/reject runnable will be dispatched. Null out our refcounted
// this-value now so that it's released predictably on the dispatch thread.
mThisVal = nullptr;
}
protected:
virtual void DoResolve(ResolveValueType aResolveValue) MOZ_OVERRIDE
{
Consumer::mComplete = true;
if (Consumer::mDisconnected) {
MOZ_ASSERT(!mThisVal);
PROMISE_LOG("ThenValue::DoResolve disconnected - bailing out [this=%p]", this);
// Null these out for the same reasons described below.
mResponseTarget = nullptr;
mThisVal = nullptr;
return;
}
InvokeCallbackMethod(mThisVal.get(), mResolveMethod, aResolveValue);
// Null these out after invoking the callback so that any references are
// released predictably on the target thread. Otherwise, they would be
// Null out mThisVal after invoking the callback so that any references are
// released predictably on the dispatch thread. Otherwise, it would be
// released on whatever thread last drops its reference to the ThenValue,
// which may or may not be ok.
mResponseTarget = nullptr;
mThisVal = nullptr;
}
@ -288,25 +301,22 @@ protected:
{
Consumer::mComplete = true;
if (Consumer::mDisconnected) {
MOZ_ASSERT(!mThisVal);
PROMISE_LOG("ThenValue::DoReject disconnected - bailing out [this=%p]", this);
// Null these out for the same reasons described below.
mResponseTarget = nullptr;
mThisVal = nullptr;
return;
}
InvokeCallbackMethod(mThisVal.get(), mRejectMethod, aRejectValue);
// Null these out after invoking the callback so that any references are
// released predictably on the target thread. Otherwise, they would be
// Null out mThisVal after invoking the callback so that any references are
// released predictably on the dispatch thread. Otherwise, it would be
// released on whatever thread last drops its reference to the ThenValue,
// which may or may not be ok.
mResponseTarget = nullptr;
mThisVal = nullptr;
}
private:
nsRefPtr<TargetType> mResponseTarget;
nsRefPtr<ThisType> mThisVal;
nsRefPtr<TargetType> mResponseTarget; // May be released on any thread.
nsRefPtr<ThisType> mThisVal; // Only accessed and refcounted on dispatch thread.
ResolveMethodType mResolveMethod;
RejectMethodType mRejectMethod;
};
@ -661,7 +671,7 @@ ProxyInternal(TargetType* aTarget, MethodCallBase<PromiseType>* aMethodCall, con
nsRefPtr<ProxyRunnable<PromiseType>> r = new ProxyRunnable<PromiseType>(p, aMethodCall);
nsresult rv = detail::DispatchMediaPromiseRunnable(aTarget, r);
MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
(void) rv; // Avoid compilation failures in builds with MOZ_DIAGNOSTIC_ASSERT disabled.
unused << rv;
return Move(p);
}