Bug 1237842 - Unlock mMutex before calling CloseActive. r=cpearce
authorGerald Squelart <gsquelart@mozilla.com>
Tue, 12 Jan 2016 21:22:00 +0100
changeset 280034 6a44d73430aa0d1d4e6b37b4db61a93a933832f8
parent 280033 e97ba0de7c958531335c973f234fe618463b9c7b
child 280035 910229c7409913ed37ecff8a350a1631e60f5662
push id29900
push usercbook@mozilla.com
push dateFri, 15 Jan 2016 10:47:20 +0000
treeherdermozilla-central@e1486d83107f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1237842
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1237842 - Unlock mMutex before calling CloseActive. r=cpearce UnloadPlugins runs in the GMP thread. While it locks mMutex, any locking attempt on the main thread would effectively block the main thread while we are calling CloseActive on each plugin. The problem is that CloseActive calls GeckoMediaServiceParent::GetSingleton(), which dispatches a task to the main thread, which may be blocked because of mMutex. To solve this inter-locking issue, the list of plugins is first copied to a local array, after which mMutex may be released, and we may now call CloseActive without risking locking.
dom/media/gmp/GMPServiceParent.cpp
--- a/dom/media/gmp/GMPServiceParent.cpp
+++ b/dom/media/gmp/GMPServiceParent.cpp
@@ -646,40 +646,44 @@ GeckoMediaPluginServiceParent::UnloadPlu
   MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
   MOZ_ASSERT(!mShuttingDownOnGMPThread);
   mShuttingDownOnGMPThread = true;
 #ifdef MOZ_CRASHREPORTER
       SetAsyncShutdownPluginState(nullptr, '2',
         NS_LITERAL_CSTRING("Starting to unload plugins"));
 #endif
 
+  nsTArray<RefPtr<GMPParent>> plugins;
   {
     MutexAutoLock lock(mMutex);
-    LOGD(("%s::%s plugins:%u including async:%u", __CLASS__, __FUNCTION__,
-          mPlugins.Length(), mAsyncShutdownPlugins.Length()));
+    // Move all plugins references to a local array. This way mMutex won't be
+    // locked when calling CloseActive (to avoid inter-locking).
+    plugins = Move(mPlugins);
+  }
+
+  LOGD(("%s::%s plugins:%u including async:%u", __CLASS__, __FUNCTION__,
+        plugins.Length(), mAsyncShutdownPlugins.Length()));
 #ifdef DEBUG
-    for (const auto& plugin : mPlugins) {
-      LOGD(("%s::%s plugin: '%s'", __CLASS__, __FUNCTION__,
-            plugin->GetDisplayName().get()));
-    }
-    for (const auto& plugin : mAsyncShutdownPlugins) {
-      LOGD(("%s::%s async plugin: '%s'", __CLASS__, __FUNCTION__,
-            plugin->GetDisplayName().get()));
-    }
+  for (const auto& plugin : plugins) {
+    LOGD(("%s::%s plugin: '%s'", __CLASS__, __FUNCTION__,
+          plugin->GetDisplayName().get()));
+  }
+  for (const auto& plugin : mAsyncShutdownPlugins) {
+    LOGD(("%s::%s async plugin: '%s'", __CLASS__, __FUNCTION__,
+          plugin->GetDisplayName().get()));
+  }
 #endif
-    // Note: CloseActive may be async; it could actually finish
-    // shutting down when all the plugins have unloaded.
-    for (size_t i = 0; i < mPlugins.Length(); i++) {
+  // Note: CloseActive may be async; it could actually finish
+  // shutting down when all the plugins have unloaded.
+  for (const auto& plugin : plugins) {
 #ifdef MOZ_CRASHREPORTER
-      SetAsyncShutdownPluginState(mPlugins[i], 'S',
-          NS_LITERAL_CSTRING("CloseActive"));
+    SetAsyncShutdownPluginState(plugin, 'S',
+        NS_LITERAL_CSTRING("CloseActive"));
 #endif
-      mPlugins[i]->CloseActive(true);
-    }
-    mPlugins.Clear();
+    plugin->CloseActive(true);
   }
 
 #ifdef MOZ_CRASHREPORTER
       SetAsyncShutdownPluginState(nullptr, '3',
         NS_LITERAL_CSTRING("Dispatching sync-shutdown-complete"));
 #endif
   nsCOMPtr<nsIRunnable> task(NS_NewRunnableMethod(
     this, &GeckoMediaPluginServiceParent::NotifySyncShutdownComplete));