Bug 1281090. Part 5 - don't lock while sending synchronous notification to avoid deadlock. r=gerald.
MozReview-Commit-ID: hOasuXLf5a
--- a/dom/media/MediaEventSource.h
+++ b/dom/media/MediaEventSource.h
@@ -491,32 +491,61 @@ public:
MediaEventListener
Connect(nsIEventTarget* aTarget, This* aThis, Method aMethod) {
return ConnectInternal(aTarget, aThis, aMethod);
}
protected:
MediaEventSourceImpl() : mMutex("MediaEventSourceImpl::mMutex") {}
- template <typename... Ts>
- void NotifyInternal(Ts&&... aEvents) {
+ template <DispatchPolicy P, typename... Ts>
+ typename EnableIf<P == DispatchPolicy::Async, void>::Type
+ NotifyInternal(IntegralConstant<DispatchPolicy, P>, Ts&&... aEvents) {
MutexAutoLock lock(mMutex);
int32_t last = static_cast<int32_t>(mListeners.Length()) - 1;
for (int32_t i = last; i >= 0; --i) {
auto&& l = mListeners[i];
// Remove disconnected listeners.
// It is not optimal but is simple and works well.
if (l->Token()->IsRevoked()) {
mListeners.RemoveElementAt(i);
continue;
}
l->Dispatch(Forward<Ts>(aEvents)...);
}
}
+ template <DispatchPolicy P, typename... Ts>
+ typename EnableIf<P == DispatchPolicy::Sync, void>::Type
+ NotifyInternal(IntegralConstant<DispatchPolicy, P>, Ts&&... aEvents) {
+ // Move |mListeners| to a new container before iteration to prevent
+ // |mListeners| from being disrupted if the listener calls Connect() to
+ // modify |mListeners| in the callback function.
+ nsTArray<UniquePtr<Listener>> listeners;
+ listeners.SwapElements(mListeners);
+ for (auto&& l : listeners) {
+ l->Dispatch(Forward<Ts>(aEvents)...);
+ }
+ PruneListeners();
+ // Move remaining listeners back to |mListeners|.
+ for (auto&& l : listeners) {
+ if (!l->Token()->IsRevoked()) {
+ mListeners.AppendElement(Move(l));
+ }
+ }
+ // Perform sanity checks.
+ MOZ_ASSERT(Lp == ListenerPolicy::NonExclusive || mListeners.Length() <= 1);
+ }
+
+ template <typename... Ts>
+ void Notify(Ts&&... aEvents) {
+ NotifyInternal(IntegralConstant<DispatchPolicy, Dp>(),
+ Forward<Ts>(aEvents)...);
+ }
+
private:
Mutex mMutex;
nsTArray<UniquePtr<Listener>> mListeners;
};
template <typename... Es>
using MediaEventSource =
MediaEventSourceImpl<DispatchPolicy::Async,
@@ -529,71 +558,61 @@ using MediaEventSourceExc =
/**
* A class to separate the interface of event subject (MediaEventSource)
* and event publisher. Mostly used as a member variable to publish events
* to the listeners.
*/
template <typename... Es>
class MediaEventProducer : public MediaEventSource<Es...> {
public:
- template <typename... Ts>
- void Notify(Ts&&... aEvents) {
- this->NotifyInternal(Forward<Ts>(aEvents)...);
- }
+ using MediaEventSource<Es...>::Notify;
};
/**
* Specialization for void type. A dummy bool is passed to NotifyInternal
* since there is no way to pass a void value.
*/
template <>
class MediaEventProducer<void> : public MediaEventSource<void> {
public:
void Notify() {
- this->NotifyInternal(true /* dummy */);
+ MediaEventSource<void>::Notify(false /* dummy */);
}
};
/**
* A producer allowing at most one listener.
*/
template <typename... Es>
class MediaEventProducerExc : public MediaEventSourceExc<Es...> {
public:
- template <typename... Ts>
- void Notify(Ts&&... aEvents) {
- this->NotifyInternal(Forward<Ts>(aEvents)...);
- }
+ using MediaEventSourceExc<Es...>::Notify;
};
/**
* Events are passed directly to the callback function of the listeners without
* dispatching. Note this class is not thread-safe. Both Connect() and Notify()
* must be called on the same thread.
*/
template <typename... Es>
class MediaCallback
: public MediaEventSourceImpl<DispatchPolicy::Sync,
ListenerPolicy::NonExclusive, Es...> {
public:
- template <typename... Ts>
- void Notify(Ts&&... aEvents) {
- this->NotifyInternal(Forward<Ts>(aEvents)...);
- }
+ using MediaEventSourceImpl<DispatchPolicy::Sync,
+ ListenerPolicy::NonExclusive, Es...>::Notify;
};
/**
* A special version of MediaCallback which allows at most one listener.
*/
template <typename... Es>
class MediaCallbackExc
: public MediaEventSourceImpl<DispatchPolicy::Sync,
ListenerPolicy::Exclusive, Es...> {
public:
- template <typename... Ts>
- void Notify(Ts&&... aEvents) {
- this->NotifyInternal(Forward<Ts>(aEvents)...);
- }
+ using MediaEventSourceImpl<DispatchPolicy::Sync,
+ ListenerPolicy::Exclusive, Es...>::Notify;
};
} // namespace mozilla
#endif //MediaEventSource_h_