Bug 1039572 - Fix the re-entry problem with GMPParent::ActorDestroy and the MaybeUnload callers who are asynchronous, r=jesup
authorBenjamin Smedberg <benjamin@smedbergs.us>
Fri, 18 Jul 2014 13:37:06 -0400
changeset 216879 ad15134168117f116363e1174c0846465f2d9f15
parent 216878 55762bee8f273e5b24caa8d69de97f353c8e0758
child 216880 e805b9fb4758ad740757c0f667bd6204e3f16e9a
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)
reviewersjesup
bugs1039572
milestone33.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 1039572 - Fix the re-entry problem with GMPParent::ActorDestroy and the MaybeUnload callers who are asynchronous, r=jesup
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
@@ -56,20 +56,17 @@ GMPParent::Init(nsIFile* aPluginDir)
   return ReadGMPMetaData();
 }
 
 nsresult
 GMPParent::LoadProcess()
 {
   MOZ_ASSERT(mDirectory, "Plugin directory cannot be NULL!");
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
-
-  if (mState == GMPStateLoaded) {
-    return NS_OK;
-  }
+  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)) {
@@ -86,80 +83,91 @@ GMPParent::LoadProcess()
   }
 
   mState = GMPStateLoaded;
 
   return NS_OK;
 }
 
 void
-GMPParent::MaybeUnloadProcess()
+GMPParent::CloseIfUnused()
 {
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
-  if (mVideoDecoders.Length() == 0 &&
+  if (mState == GMPStateLoaded &&
+      mVideoDecoders.Length() == 0 &&
       mVideoEncoders.Length() == 0) {
-    UnloadProcess();
+    Shutdown();
   }
 }
 
 void
-GMPParent::UnloadProcess()
+GMPParent::CloseActive()
 {
-  MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
-
-  if (mState == GMPStateNotLoaded) {
-    MOZ_ASSERT(mVideoDecoders.IsEmpty() && mVideoEncoders.IsEmpty());
-    return;
-  }
-
-  mState = GMPStateNotLoaded;
-
   // Invalidate and remove any remaining API objects.
   for (uint32_t i = mVideoDecoders.Length(); i > 0; i--) {
     mVideoDecoders[i - 1]->DecodingComplete();
   }
 
   // Invalidate and remove any remaining API objects.
   for (uint32_t i = mVideoEncoders.Length(); i > 0; i--) {
     mVideoEncoders[i - 1]->EncodingComplete();
   }
+}
 
+void
+GMPParent::Shutdown()
+{
+  MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
+
+  if (mState == GMPStateNotLoaded || mState == GMPStateClosing) {
+    MOZ_ASSERT(mVideoDecoders.IsEmpty() && mVideoEncoders.IsEmpty());
+    return;
+  }
+
+  CloseActive();
   Close();
-  if (mProcess) {
-    mProcess->Delete();
-    mProcess = nullptr;
-  }
+  DeleteProcess();
+  MOZ_ASSERT(mState == GMPStateNotLoaded);
+}
+
+void
+GMPParent::DeleteProcess()
+{
+  MOZ_ASSERT(mState == GMPStateClosing);
+  mProcess->Delete();
+  mProcess = nullptr;
+  mState = GMPStateNotLoaded;
 }
 
 void
 GMPParent::VideoDecoderDestroyed(GMPVideoDecoderParent* aDecoder)
 {
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
   // If the constructor fails, we'll get called before it's added
   unused << NS_WARN_IF(!mVideoDecoders.RemoveElement(aDecoder));
 
   // Recv__delete__ is on the stack, don't potentially destroy the top-level actor
   // until after this has completed.
-  nsCOMPtr<nsIRunnable> event = NS_NewRunnableMethod(this, &GMPParent::MaybeUnloadProcess);
+  nsCOMPtr<nsIRunnable> event = NS_NewRunnableMethod(this, &GMPParent::CloseIfUnused);
   NS_DispatchToCurrentThread(event);
 }
 
 void
 GMPParent::VideoEncoderDestroyed(GMPVideoEncoderParent* aEncoder)
 {
   MOZ_ASSERT(GMPThread() == NS_GetCurrentThread());
 
   // If the constructor fails, we'll get called before it's added
   unused << NS_WARN_IF(!mVideoEncoders.RemoveElement(aEncoder));
 
   // Recv__delete__ is on the stack, don't potentially destroy the top-level actor
   // until after this has completed.
-  nsCOMPtr<nsIRunnable> event = NS_NewRunnableMethod(this, &GMPParent::MaybeUnloadProcess);
+  nsCOMPtr<nsIRunnable> event = NS_NewRunnableMethod(this, &GMPParent::CloseIfUnused);
   NS_DispatchToCurrentThread(event);
 }
 
 GMPState
 GMPParent::State() const
 {
   return mState;
 }
@@ -200,16 +208,19 @@ GMPParent::SupportsAPI(const nsCString& 
 }
 
 bool
 GMPParent::EnsureProcessLoaded()
 {
   if (mState == GMPStateLoaded) {
     return true;
   }
+  if (mState == GMPStateClosing) {
+    return false;
+  }
 
   nsresult rv = LoadProcess();
 
   return NS_SUCCEEDED(rv);
 }
 
 nsresult
 GMPParent::GetGMPVideoDecoder(GMPVideoDecoderParent** aGMPVD)
@@ -284,24 +295,31 @@ GMPParent::GetCrashID(nsString& aResult)
   GetIDFromMinidump(dumpFile, aResult);
   cr->GenerateCrashReportForMinidump(dumpFile, &notes);
 }
 #endif
 
 void
 GMPParent::ActorDestroy(ActorDestroyReason aWhy)
 {
+  mState = GMPStateClosing;
   if (AbnormalShutdown == aWhy) {
     nsString dumpID;
 #ifdef MOZ_CRASHREPORTER
     GetCrashID(dumpID);
 #endif
     // now do something with the crash ID, bug 1038961
   }
-  UnloadProcess();
+
+  CloseActive();
+
+  // Normal Shutdown() will delete the process on unwind.
+  if (AbnormalShutdown == aWhy) {
+    NS_DispatchToCurrentThread(NS_NewRunnableMethod(this, &GMPParent::DeleteProcess));
+  }
 }
 
 mozilla::dom::PCrashReporterParent*
 GMPParent::AllocPCrashReporterParent(const NativeThreadId& aThread)
 {
 #ifndef MOZ_CRASHREPORTER
   MOZ_ASSERT(false, "Should only be sent if crash reporting is enabled.");
 #endif
--- a/content/media/gmp/GMPParent.h
+++ b/content/media/gmp/GMPParent.h
@@ -39,30 +39,43 @@ class GMPCapability
 {
 public:
   nsCString mAPIName;
   nsTArray<nsCString> mAPITags;
 };
 
 enum GMPState {
   GMPStateNotLoaded,
-  GMPStateLoaded
+  GMPStateLoaded,
+  GMPStateClosing
 };
 
 class GMPParent MOZ_FINAL : public PGMPParent
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING_WITH_MAIN_THREAD_DESTRUCTION(GMPParent)
 
   GMPParent();
 
   nsresult Init(nsIFile* aPluginDir);
   nsresult LoadProcess();
-  void MaybeUnloadProcess();
-  void UnloadProcess();
+
+  // 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();
+
+  // 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);
   nsresult GetGMPVideoDecoder(GMPVideoDecoderParent** aGMPVD);
   void VideoDecoderDestroyed(GMPVideoDecoderParent* aDecoder);
   nsresult GetGMPVideoEncoder(GMPVideoEncoderParent** aGMPVE);
   void VideoEncoderDestroyed(GMPVideoEncoderParent* aEncoder);
   GMPState State() const;
 #ifdef DEBUG
   nsIThread* GMPThread();
--- a/content/media/gmp/GMPService.cpp
+++ b/content/media/gmp/GMPService.cpp
@@ -257,17 +257,17 @@ GeckoMediaPluginService::UnloadPlugins()
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
 
   MOZ_ASSERT(!mShuttingDownOnGMPThread);
   mShuttingDownOnGMPThread = true;
 
   MutexAutoLock lock(mMutex);
   for (uint32_t i = 0; i < mPlugins.Length(); i++) {
-    mPlugins[i]->UnloadProcess();
+    mPlugins[i]->Shutdown();
   }
   mPlugins.Clear();
 }
 
 void
 GeckoMediaPluginService::LoadFromEnvironment()
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mGMPThread);
@@ -438,17 +438,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]->UnloadProcess();
+      mPlugins[i]->Shutdown();
       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."));
 }