Bug 1041232: Deferred GMP process shutdown to avoid Shmem lockup (workaround) r=cpearce a=sylvestre
authorRandell Jesup <rjesup@jesup.org>
Thu, 24 Jul 2014 21:47:41 -0400
changeset 217307 0cc303c8d18ad599b07d909b9cd9bee139583299
parent 217306 3fb159eeaad747ec7e2b1722e91117625240e335
child 217308 a107c16ebe66575e19f92dced2dbe22cd0ec4e46
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sylvestre
bugs1041232
milestone33.0a2
Bug 1041232: Deferred GMP process shutdown to avoid Shmem lockup (workaround) r=cpearce a=sylvestre
content/media/gmp/GMPParent.cpp
content/media/gmp/GMPParent.h
content/media/gmp/GMPService.cpp
--- a/content/media/gmp/GMPParent.cpp
+++ b/content/media/gmp/GMPParent.cpp
@@ -26,21 +26,24 @@ using CrashReporter::GetIDFromMinidump;
 #endif
 
 namespace mozilla {
 namespace gmp {
 
 GMPParent::GMPParent()
   : mState(GMPStateNotLoaded)
   , mProcess(nullptr)
+  , mDeleteProcessOnUnload(false)
 {
 }
 
 GMPParent::~GMPParent()
 {
+  // Can't Close or Destroy the process here, since destruction is MainThread only
+  MOZ_ASSERT(NS_IsMainThread());
 }
 
 nsresult
 GMPParent::Init(nsIFile* aPluginDir)
 {
   MOZ_ASSERT(aPluginDir);
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
@@ -65,51 +68,57 @@ GMPParent::LoadProcess()
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
   MOZ_ASSERT(mState == GMPStateNotLoaded);
 
   nsAutoCString path;
   if (NS_FAILED(mDirectory->GetNativePath(path))) {
     return NS_ERROR_FAILURE;
   }
 
-  mProcess = new GMPProcessParent(path.get());
-  if (!mProcess->Launch(30 * 1000)) {
-    mProcess->Delete();
-    mProcess = nullptr;
-    return NS_ERROR_FAILURE;
-  }
+  if (!mProcess) {
+    mProcess = new GMPProcessParent(path.get());
+    if (!mProcess->Launch(30 * 1000)) {
+      mProcess->Delete();
+      mProcess = nullptr;
+      return NS_ERROR_FAILURE;
+    }
 
-  bool opened = Open(mProcess->GetChannel(), mProcess->GetChildProcessHandle());
-  if (!opened) {
-    mProcess->Delete();
-    mProcess = nullptr;
-    return NS_ERROR_FAILURE;
+    bool opened = Open(mProcess->GetChannel(), mProcess->GetChildProcessHandle());
+    if (!opened) {
+      mProcess->Delete();
+      mProcess = nullptr;
+      return NS_ERROR_FAILURE;
+    }
   }
 
   mState = GMPStateLoaded;
 
   return NS_OK;
 }
 
 void
 GMPParent::CloseIfUnused()
 {
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
-  if ((mState == GMPStateLoaded ||
+  if ((mDeleteProcessOnUnload ||
+       mState == GMPStateLoaded ||
        mState == GMPStateUnloading) &&
       mVideoDecoders.IsEmpty() &&
       mVideoEncoders.IsEmpty()) {
     Shutdown();
   }
 }
 
 void
-GMPParent::CloseActive()
+GMPParent::CloseActive(bool aDieWhenUnloaded)
 {
+  if (aDieWhenUnloaded) {
+    mDeleteProcessOnUnload = true; // don't allow this to go back...
+  }
   if (mState == GMPStateLoaded) {
     mState = GMPStateUnloading;
   }
 
   // Invalidate and remove any remaining API objects.
   for (uint32_t i = mVideoDecoders.Length(); i > 0; i--) {
     mVideoDecoders[i - 1]->Shutdown();
   }
@@ -130,26 +139,31 @@ GMPParent::Shutdown()
 {
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
   MOZ_ASSERT(mVideoDecoders.IsEmpty() && mVideoEncoders.IsEmpty());
   if (mState == GMPStateNotLoaded || mState == GMPStateClosing) {
     return;
   }
 
-  mState = GMPStateClosing;
-  Close();
-  DeleteProcess();
+  // XXX Get rid of mDeleteProcessOnUnload and do this on all Shutdowns once
+  // Bug 1043671 is fixed (backout this patch)
+  if (mDeleteProcessOnUnload) {
+    mState = GMPStateClosing;
+    DeleteProcess();
+  } else {
+    mState = GMPStateNotLoaded;
+  }
   MOZ_ASSERT(mState == GMPStateNotLoaded);
 }
 
 void
 GMPParent::DeleteProcess()
 {
-  MOZ_ASSERT(mState == GMPStateClosing);
+  Close();
   mProcess->Delete();
   mProcess = nullptr;
   mState = GMPStateNotLoaded;
 }
 
 void
 GMPParent::VideoDecoderDestroyed(GMPVideoDecoderParent* aDecoder)
 {
@@ -357,17 +371,17 @@ GMPParent::ActorDestroy(ActorDestroyReas
 
     // NotifyObservers is mainthread-only
     NS_DispatchToMainThread(WrapRunnableNM(&GMPNotifyObservers, id),
                             NS_DISPATCH_NORMAL);
   }
 #endif
   // warn us off trying to close again
   mState = GMPStateClosing;
-  CloseActive();
+  CloseActive(false);
 
   // Normal Shutdown() will delete the process on unwind.
   if (AbnormalShutdown == aWhy) {
     NS_DispatchToCurrentThread(NS_NewRunnableMethod(this, &GMPParent::DeleteProcess));
   }
 }
 
 mozilla::dom::PCrashReporterParent*
--- a/content/media/gmp/GMPParent.h
+++ b/content/media/gmp/GMPParent.h
@@ -59,17 +59,17 @@ public:
   nsresult Init(nsIFile* aPluginDir);
   nsresult LoadProcess();
 
   // Called internally to close this if we don't need it
   void CloseIfUnused();
 
   // Notify all active de/encoders that we are closing, either because of
   // normal shutdown or unexpected shutdown/crash.
-  void CloseActive();
+  void CloseActive(bool aDieWhenUnloaded);
 
   // Called by the GMPService to forcibly close active de/encoders at shutdown
   void Shutdown();
 
   // This must not be called while we're in the middle of abnormal ActorDestroy
   void DeleteProcess();
 
   bool SupportsAPI(const nsCString& aAPI, const nsCString& aTag);
@@ -127,16 +127,17 @@ private:
   GMPState mState;
   nsCOMPtr<nsIFile> mDirectory; // plugin directory on disk
   nsString mName; // base name of plugin on disk, UTF-16 because used for paths
   nsCString mDisplayName; // name of plugin displayed to users
   nsCString mDescription; // description of plugin for display to users
   nsCString mVersion;
   nsTArray<nsAutoPtr<GMPCapability>> mCapabilities;
   GMPProcessParent* mProcess;
+  bool mDeleteProcessOnUnload;
 
   nsTArray<nsRefPtr<GMPVideoDecoderParent>> mVideoDecoders;
   nsTArray<nsRefPtr<GMPVideoEncoderParent>> mVideoEncoders;
 #ifdef DEBUG
   nsCOMPtr<nsIThread> mGMPThread;
 #endif
   // Origin the plugin is assigned to, or empty if the the plugin is not
   // assigned to an origin.
--- a/content/media/gmp/GMPService.cpp
+++ b/content/media/gmp/GMPService.cpp
@@ -260,17 +260,17 @@ GeckoMediaPluginService::UnloadPlugins()
 
   MOZ_ASSERT(!mShuttingDownOnGMPThread);
   mShuttingDownOnGMPThread = true;
 
   MutexAutoLock lock(mMutex);
   // Note: CloseActive is async; it will actually finish
   // shutting down when all the plugins have unloaded.
   for (uint32_t i = 0; i < mPlugins.Length(); i++) {
-    mPlugins[i]->CloseActive();
+    mPlugins[i]->CloseActive(true);
   }
   mPlugins.Clear();
 }
 
 void
 GeckoMediaPluginService::LoadFromEnvironment()
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
@@ -441,17 +441,17 @@ GeckoMediaPluginService::RemoveOnGMPThre
     return;
   }
 
   MutexAutoLock lock(mMutex);
   for (uint32_t i = 0; i < mPlugins.Length(); ++i) {
     nsCOMPtr<nsIFile> pluginpath = mPlugins[i]->GetDirectory();
     bool equals;
     if (NS_SUCCEEDED(directory->Equals(pluginpath, &equals)) && equals) {
-      mPlugins[i]->CloseActive();
+      mPlugins[i]->CloseActive(true);
       mPlugins.RemoveElementAt(i);
       return;
     }
   }
   NS_WARNING("Removing GMP which was never added.");
   nsCOMPtr<nsIConsoleService> cs = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
   cs->LogStringMessage(MOZ_UTF16("Removing GMP which was never added."));
 }