Bug 1469603 - Use a recursive lock in crash reporter callbacks that might be called synchronously. r=erahm, a=IanN CLOSED TREE DONTBUILD SEAMONKEY_2_49_ESR_RELBRANCH
authorGabriele Svelto <gsvelto@mozilla.com>
Thu, 21 Jun 2018 09:56:26 +0200
branchSEAMONKEY_2_49_ESR_RELBRANCH
changeset 357513 e4802812e965cdb2418a6783d5b48bf4274661c9
parent 357512 8265d50c2c1b91e5db2121867a46589534d65f16
child 357514 49b114b42facc7fbd9ec8c9aaa140e7521fe0952
push id7834
push userfrgrahl@gmx.net
push dateSun, 13 Jan 2019 12:17:02 +0000
treeherdermozilla-esr52@6e4ad8a8f2e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm, IanN
bugs1469603
milestone52.9.1
Bug 1469603 - Use a recursive lock in crash reporter callbacks that might be called synchronously. r=erahm, a=IanN CLOSED TREE DONTBUILD mozilla-esr52 SEAMONKEY_2_49_ESR_RELBRANCH
dom/plugins/ipc/PluginModuleParent.cpp
dom/plugins/ipc/PluginModuleParent.h
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -590,17 +590,17 @@ PluginModuleChromeParent::OnProcessLaunc
     if (crashReporter) {
         crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("AsyncPluginInit"),
                                            mIsStartingAsync ?
                                                NS_LITERAL_CSTRING("1") :
                                                NS_LITERAL_CSTRING("0"));
     }
 #ifdef XP_WIN
     { // Scope for lock
-        mozilla::MutexAutoLock lock(mCrashReporterMutex);
+        ReentrantMonitorAutoEnter mon(mCrashReporterMutex);
         mCrashReporter = CrashReporter();
     }
 #endif
 #endif
 
 #if defined(XP_WIN) && defined(_X86_)
     // Protected mode only applies to Windows and only to x86.
     if (!mIsBlocklisted && mIsFlashPlugin &&
@@ -819,17 +819,17 @@ PluginModuleChromeParent::~PluginModuleC
 }
 
 #ifdef MOZ_CRASHREPORTER
 void
 PluginModuleChromeParent::WriteExtraDataForMinidump(AnnotationTable& notes)
 {
 #ifdef XP_WIN
     // mCrashReporterMutex is already held by the caller
-    mCrashReporterMutex.AssertCurrentThreadOwns();
+    mCrashReporterMutex.AssertCurrentThreadIn();
 #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;
@@ -1181,17 +1181,17 @@ PluginModuleContentParent::OnExitedSyncS
 
 void
 PluginModuleChromeParent::TakeFullMinidump(base::ProcessId aContentPid,
                                            const nsAString& aBrowserDumpId,
                                            nsString& aDumpId)
 {
 #ifdef MOZ_CRASHREPORTER
 #ifdef XP_WIN
-    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    ReentrantMonitorAutoEnter mon(mCrashReporterMutex);
 #endif // XP_WIN
 
     CrashReporterParent* crashReporter = CrashReporter();
     if (!crashReporter) {
         return;
     }
 
     bool reportsReady = false;
@@ -1278,17 +1278,17 @@ PluginModuleChromeParent::TerminateChild
     // because it also needs to lock the mCrashReporterMutex and Mutex doesn't
     // support recrusive locking.
     nsAutoString dumpId;
     if (aDumpId.IsEmpty()) {
         TakeFullMinidump(aContentPid, EmptyString(), dumpId);
     }
 
 #ifdef XP_WIN
-    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    ReentrantMonitorAutoEnter mon(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();
@@ -1504,17 +1504,17 @@ RemoveMinidump(nsIFile* minidump)
     }
 }
 #endif // MOZ_CRASHREPORTER_INJECTOR
 
 void
 PluginModuleChromeParent::ProcessFirstMinidump()
 {
 #ifdef XP_WIN
-    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    ReentrantMonitorAutoEnter mon(mCrashReporterMutex);
 #endif
     CrashReporterParent* crashReporter = CrashReporter();
     if (!crashReporter)
         return;
 
     AnnotationTable notes(4);
     WriteExtraDataForMinidump(notes);
 
@@ -2962,17 +2962,17 @@ PluginModuleChromeParent::AllocPCrashRep
 #endif
 }
 
 bool
 PluginModuleChromeParent::DeallocPCrashReporterParent(PCrashReporterParent* actor)
 {
 #ifdef MOZ_CRASHREPORTER
 #ifdef XP_WIN
-    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    ReentrantMonitorAutoEnter mon(mCrashReporterMutex);
     if (actor == static_cast<PCrashReporterParent*>(mCrashReporter)) {
         mCrashReporter = nullptr;
     }
 #endif
 #endif
     delete actor;
     return true;
 }
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@@ -11,16 +11,17 @@
 #include "mozilla/FileUtils.h"
 #include "mozilla/HangAnnotations.h"
 #include "mozilla/PluginLibrary.h"
 #include "mozilla/plugins/PluginProcessParent.h"
 #include "mozilla/plugins/PPluginModuleParent.h"
 #include "mozilla/plugins/PluginMessageUtils.h"
 #include "mozilla/plugins/PluginTypes.h"
 #include "mozilla/ipc/TaskFactory.h"
+#include "mozilla/ReentrantMonitor.h"   // for ReentrantMonitorAutoEnter, etc
 #include "mozilla/TimeStamp.h"
 #include "npapi.h"
 #include "npfunctions.h"
 #include "nsDataHashtable.h"
 #include "nsHashKeys.h"
 #include "nsIObserver.h"
 #ifdef XP_WIN
 #include "nsWindowsHelpers.h"
@@ -503,16 +504,17 @@ class PluginModuleChromeParent
 
     virtual bool
     RecvProfile(const nsCString& aProfile) override;
 
     virtual bool
     AnswerGetKeyState(const int32_t& aVirtKey, int16_t* aRet) override;
 
 private:
+    typedef mozilla::ReentrantMonitor ReentrantMonitor;
     virtual void
     EnteredCxxStack() override;
 
     void
     ExitedCxxStack() override;
 
     mozilla::ipc::IProtocol* GetInvokingProtocol();
     PluginInstanceParent* GetManagingInstance(mozilla::ipc::IProtocol* aProtocol);
@@ -594,19 +596,21 @@ private:
     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.
+     * that mCrashReporter refers to. Sometimes asynchronous crash reporter
+     * callbacks are dispatched synchronously while the caller is still holding
+     * the mutex. This requires recursive locking support in the mutex.
      */
-    mozilla::Mutex mCrashReporterMutex;
+    ReentrantMonitor mCrashReporterMutex;
     CrashReporterParent* mCrashReporter;
 #endif // MOZ_CRASHREPORTER
 
 
     /**
      * Launches the Plugin Hang UI.
      *
      * @return true if plugin-hang-ui.exe has been successfully launched.