From 592239bbd9717e8b23f114f78f922072900fc77d Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 16 Apr 2015 09:20:22 -0700 Subject: [PATCH] Bug 1154802 - Assert against potential deadlocks between synchronous MediaTaskQueue operations and tail dispatchers. r=jww --- dom/media/MediaTaskQueue.cpp | 27 +++++++++++++++++++++++++-- dom/media/TaskDispatcher.h | 23 +++++++++++++++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/dom/media/MediaTaskQueue.cpp b/dom/media/MediaTaskQueue.cpp index f5dae5c0ae20..11231fb31150 100644 --- a/dom/media/MediaTaskQueue.cpp +++ b/dom/media/MediaTaskQueue.cpp @@ -106,8 +106,16 @@ private: void MediaTaskQueue::SyncDispatch(TemporaryRef aRunnable) { NS_WARNING("MediaTaskQueue::SyncDispatch is dangerous and deprecated. Stop using this!"); - RefPtr task(new MediaTaskQueueSyncRunnable(aRunnable)); - Dispatch(task); + nsRefPtr task(new MediaTaskQueueSyncRunnable(aRunnable)); + + // Tail dispatchers don't interact nicely with sync dispatch. We require that + // nothing is already in the tail dispatcher, and then sidestep it for this + // task. + MOZ_ASSERT_IF(AbstractThread::GetCurrent(), + !AbstractThread::GetCurrent()->TailDispatcher().HasTasksFor(this)); + nsRefPtr taskRef = task; + Dispatch(taskRef.forget(), AssertDispatchSuccess, TailDispatch); + task->WaitUntilDone(); } @@ -121,6 +129,11 @@ MediaTaskQueue::AwaitIdle() void MediaTaskQueue::AwaitIdleLocked() { + // Make the there are no tasks for this queue waiting in the caller's tail + // dispatcher. + MOZ_ASSERT_IF(AbstractThread::GetCurrent(), + !AbstractThread::GetCurrent()->TailDispatcher().HasTasksFor(this)); + mQueueMonitor.AssertCurrentThreadOwns(); MOZ_ASSERT(mIsRunning || mTasks.empty()); while (mIsRunning) { @@ -131,6 +144,11 @@ MediaTaskQueue::AwaitIdleLocked() void MediaTaskQueue::AwaitShutdownAndIdle() { + // Make the there are no tasks for this queue waiting in the caller's tail + // dispatcher. + MOZ_ASSERT_IF(AbstractThread::GetCurrent(), + !AbstractThread::GetCurrent()->TailDispatcher().HasTasksFor(this)); + MonitorAutoLock mon(mQueueMonitor); while (!mIsShutdown) { mQueueMonitor.Wait(); @@ -176,6 +194,11 @@ FlushableMediaTaskQueue::FlushAndDispatch(TemporaryRef aRunnable) void FlushableMediaTaskQueue::FlushLocked() { + // Make the there are no tasks for this queue waiting in the caller's tail + // dispatcher. + MOZ_ASSERT_IF(AbstractThread::GetCurrent(), + !AbstractThread::GetCurrent()->TailDispatcher().HasTasksFor(this)); + mQueueMonitor.AssertCurrentThreadOwns(); MOZ_ASSERT(mIsFlushing); diff --git a/dom/media/TaskDispatcher.h b/dom/media/TaskDispatcher.h index ea6a3efc1858..be11b1a100ca 100644 --- a/dom/media/TaskDispatcher.h +++ b/dom/media/TaskDispatcher.h @@ -44,6 +44,8 @@ public: virtual void AddTask(AbstractThread* aThread, already_AddRefed aRunnable, AbstractThread::DispatchFailureHandling aFailureHandling = AbstractThread::AssertDispatchSuccess) = 0; + + virtual bool HasTasksFor(AbstractThread* aThread) = 0; }; /* @@ -88,6 +90,8 @@ public: } } + bool HasTasksFor(AbstractThread* aThread) override { return !!GetTaskGroup(aThread); } + private: struct PerThreadTaskGroup @@ -131,16 +135,27 @@ private: PerThreadTaskGroup& EnsureTaskGroup(AbstractThread* aThread) { - for (size_t i = 0; i < mTaskGroups.Length(); ++i) { - if (mTaskGroups[i]->mThread == aThread) { - return *mTaskGroups[i]; - } + PerThreadTaskGroup* existing = GetTaskGroup(aThread); + if (existing) { + return *existing; } mTaskGroups.AppendElement(new PerThreadTaskGroup(aThread)); return *mTaskGroups.LastElement(); } + PerThreadTaskGroup* GetTaskGroup(AbstractThread* aThread) + { + for (size_t i = 0; i < mTaskGroups.Length(); ++i) { + if (mTaskGroups[i]->mThread == aThread) { + return mTaskGroups[i].get(); + } + } + + // Not found. + return nullptr; + } + // Task groups, organized by thread. nsTArray> mTaskGroups;