Bug 852164: Adds mutual exclusion between crash reporter deletion and plugin container process termination; r=bsmedberg
authorAaron Klotz <aklotz@mozilla.com>
Tue, 03 Dec 2013 15:19:58 -0700
changeset 174310 5f07f8e8583162a1f06b427e167997583eca8330
parent 174309 81096120d82fec4177f7d94d68bb405a2721d1ea
child 174311 27a9c236606b61daab8ead8ecfea7d523c3d3414
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs852164
milestone28.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 852164: Adds mutual exclusion between crash reporter deletion and plugin container process termination; r=bsmedberg
dom/plugins/ipc/PluginModuleParent.cpp
dom/plugins/ipc/PluginModuleParent.h
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -106,16 +106,20 @@ PluginModuleParent::LoadModule(const cha
     TimeoutChanged(CHILD_TIMEOUT_PREF, parent);
 
 #ifdef MOZ_CRASHREPORTER
     // If this fails, we're having IPC troubles, and we're doomed anyways.
     if (!CrashReporterParent::CreateCrashReporter(parent.get())) {
         parent->mShutdown = true;
         return nullptr;
     }
+#ifdef XP_WIN
+    mozilla::MutexAutoLock lock(parent->mCrashReporterMutex);
+    parent->mCrashReporter = parent->CrashReporter();
+#endif
 #endif
 
     return parent.forget();
 }
 
 
 PluginModuleParent::PluginModuleParent(const char* aFilePath)
     : mSubprocess(new PluginProcessParent(aFilePath))
@@ -125,16 +129,20 @@ PluginModuleParent::PluginModuleParent(c
     , mNPNIface(nullptr)
     , mPlugin(nullptr)
     , mTaskFactory(MOZ_THIS_IN_INITIALIZER_LIST())
 #ifdef XP_WIN
     , mPluginCpuUsageOnHang()
     , mHangUIParent(nullptr)
     , mHangUIEnabled(true)
     , mIsTimerReset(true)
+#ifdef MOZ_CRASHREPORTER
+    , mCrashReporterMutex("PluginModuleParent::mCrashReporterMutex")
+    , mCrashReporter(nullptr)
+#endif
 #endif
 #ifdef MOZ_CRASHREPORTER_INJECTOR
     , mFlashProcess1(0)
     , mFlashProcess2(0)
 #endif
 {
     NS_ASSERTION(mSubprocess, "Out of memory!");
 
@@ -189,16 +197,20 @@ PluginModuleParent::~PluginModuleParent(
     }
 #endif
 }
 
 #ifdef MOZ_CRASHREPORTER
 void
 PluginModuleParent::WriteExtraDataForMinidump(AnnotationTable& notes)
 {
+#ifdef XP_WIN
+    // mCrashReporterMutex is already held by the caller
+    mCrashReporterMutex.AssertCurrentThreadOwns();
+#endif
     typedef nsDependentCString CS;
 
     // Get the plugin filename, try to get just the file leafname
     const std::string& pluginFile = mSubprocess->GetPluginFilePath();
     size_t filePos = pluginFile.rfind(FILE_PATH_SEPARATOR);
     if (filePos == std::string::npos)
         filePos = 0;
     else
@@ -414,17 +426,27 @@ PluginModuleParent::ShouldContinueFromRe
     TerminateChildProcess(MessageLoop::current());
     return false;
 }
 
 void
 PluginModuleParent::TerminateChildProcess(MessageLoop* aMsgLoop)
 {
 #ifdef MOZ_CRASHREPORTER
+#ifdef XP_WIN
+    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    CrashReporterParent* crashReporter = mCrashReporter;
+    if (!crashReporter) {
+        // If mCrashReporter is null then the hang has ended, the plugin module
+        // is shutting down. There's nothing to do here.
+        return;
+    }
+#else
     CrashReporterParent* crashReporter = CrashReporter();
+#endif
     crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("PluginHang"),
                                        NS_LITERAL_CSTRING("1"));
 #ifdef XP_WIN
     if (mHangUIParent) {
         unsigned int hangUIDuration = mHangUIParent->LastShowDurationMs();
         if (hangUIDuration) {
             nsPrintfCString strHangUIDuration("%u", hangUIDuration);
             crashReporter->AnnotateCrashReport(
@@ -631,16 +653,19 @@ RemoveMinidump(nsIFile* minidump)
         extraFile->Remove(true);
     }
 }
 #endif // MOZ_CRASHREPORTER_INJECTOR
 
 void
 PluginModuleParent::ProcessFirstMinidump()
 {
+#ifdef XP_WIN
+    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+#endif
     CrashReporterParent* crashReporter = CrashReporter();
     if (!crashReporter)
         return;
 
     AnnotationTable notes(4);
     WriteExtraDataForMinidump(notes);
 
     if (!mPluginDumpID.IsEmpty()) {
@@ -1530,16 +1555,22 @@ PluginModuleParent::AllocPCrashReporterP
 #else
     return nullptr;
 #endif
 }
 
 bool
 PluginModuleParent::DeallocPCrashReporterParent(PCrashReporterParent* actor)
 {
+#ifdef XP_WIN
+    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    if (actor == static_cast<PCrashReporterParent*>(mCrashReporter)) {
+        mCrashReporter = nullptr;
+    }
+#endif
     delete actor;
     return true;
 }
 
 bool
 PluginModuleParent::RecvSetCursor(const NSCursorInfo& aCursorInfo)
 {
     PLUGIN_LOG_DEBUG(("%s", FULLFUNCTION));
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@@ -309,16 +309,27 @@ private:
     nsString mBrowserDumpID;
     nsString mHangID;
     nsRefPtr<nsIObserver> mProfilerObserver;
 #ifdef XP_WIN
     InfallibleTArray<float> mPluginCpuUsageOnHang;
     PluginHangUIParent *mHangUIParent;
     bool mHangUIEnabled;
     bool mIsTimerReset;
+#ifdef MOZ_CRASHREPORTER
+    /**
+     * This mutex protects the crash reporter when the Plugin Hang UI event
+     * handler is executing off main thread. It is intended to protect both
+     * the mCrashReporter variable in addition to the CrashReporterParent object
+     * that mCrashReporter refers to.
+     */
+    mozilla::Mutex mCrashReporterMutex;
+    CrashReporterParent* mCrashReporter;
+#endif // MOZ_CRASHREPORTER
+
 
     void
     EvaluateHangUIState(const bool aReset);
 
     bool
     GetPluginName(nsAString& aPluginName);
 
     /**