From 371109fa034354fa1a8a9211a1f1c2bc7432e9b9 Mon Sep 17 00:00:00 2001
From: Gerald Squelart <gsquelart@mozilla.com>
Date: Wed, 27 Jul 2016 12:43:33 +1000
Subject: [PATCH] Bug 1289378 - Fix inter-locking in
 PDMFactory+GMPDecoderModule - r=jya

Say PDMFactory::EnsureInit() runs on the main thread (effectively locking it)
and then tries to lock sMonitor; if another thread was also running
EnsureInit() and locked sMonitor, it will dead-lock when trying to sync-
dispatch the GMPDecoderModule creation to the main thread.

This can be fixed by ensuring that the actual PDMFactory instance creation is
always done on the main thread (and that we don't hold sMonitor before
dispatching to the main thread.)

Note that we can now simplify GMPDecoderModule::Init and assert we are already
on the main thread.

MozReview-Commit-ID: 8xHpoymw6po

--HG--
extra : rebase_source : aabc1f4768aebdd16873bbc1afdd1679d90aed6f
---
 dom/media/platforms/PDMFactory.cpp            | 29 ++++++++++++++-----
 .../agnostic/gmp/GMPDecoderModule.cpp         |  9 +-----
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/dom/media/platforms/PDMFactory.cpp b/dom/media/platforms/PDMFactory.cpp
index e653ac8f9a1d..6d48cd1db6b6 100644
--- a/dom/media/platforms/PDMFactory.cpp
+++ b/dom/media/platforms/PDMFactory.cpp
@@ -29,6 +29,7 @@
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/SharedThreadPool.h"
 #include "mozilla/StaticPtr.h"
+#include "mozilla/SyncRunnable.h"
 #include "mozilla/TaskQueue.h"
 
 #include "MediaInfo.h"
@@ -87,17 +88,31 @@ PDMFactory::~PDMFactory()
 void
 PDMFactory::EnsureInit() const
 {
-  StaticMutexAutoLock mon(sMonitor);
-  if (!sInstance) {
-    sInstance = new PDMFactoryImpl();
+  {
+    StaticMutexAutoLock mon(sMonitor);
+    if (sInstance) {
+      // Quick exit if we already have an instance.
+      return;
+    }
     if (NS_IsMainThread()) {
+      // On the main thread and holding the lock -> Create instance.
+      sInstance = new PDMFactoryImpl();
       ClearOnShutdown(&sInstance);
-    } else {
-      nsCOMPtr<nsIRunnable> runnable =
-        NS_NewRunnableFunction([]() { ClearOnShutdown(&sInstance); });
-      NS_DispatchToMainThread(runnable);
+      return;
     }
   }
+
+  // Not on the main thread -> Sync-dispatch creation to main thread.
+  nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+  nsCOMPtr<nsIRunnable> runnable =
+    NS_NewRunnableFunction([]() {
+      StaticMutexAutoLock mon(sMonitor);
+      if (!sInstance) {
+        sInstance = new PDMFactoryImpl();
+        ClearOnShutdown(&sInstance);
+      }
+    });
+  SyncRunnable::DispatchToThread(mainThread, runnable);
 }
 
 already_AddRefed<MediaDataDecoder>
diff --git a/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp b/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
index 697b496cf938..02cdef241699 100644
--- a/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
+++ b/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
@@ -13,7 +13,6 @@
 #include "mozIGeckoMediaPluginService.h"
 #include "nsServiceManagerUtils.h"
 #include "mozilla/StaticMutex.h"
-#include "mozilla/SyncRunnable.h"
 #include "gmp-audio-decode.h"
 #include "gmp-video-decode.h"
 #ifdef XP_WIN
@@ -169,13 +168,7 @@ GMPDecoderModule::UpdateUsableCodecs()
 void
 GMPDecoderModule::Init()
 {
-  if (!NS_IsMainThread()) {
-    nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
-    nsCOMPtr<nsIRunnable> runnable =
-      NS_NewRunnableFunction([]() { Init(); });
-    SyncRunnable::DispatchToThread(mainThread, runnable);
-    return;
-  }
+  MOZ_ASSERT(NS_IsMainThread());
   // GMPService::HasPluginForAPI is main thread only, so to implement
   // SupportsMimeType() we build a table of the codecs which each whitelisted
   // GMP has and update it when any GMPs are removed or added at runtime.