Bug 791784 - Add serialization to avoid a thread race when a plug-in crashes during Firefox shutdown. r=ted, r=bsmedberg
authorArt Rothstein <mozilla-bugs@mojo-working.com>
Wed, 15 Jan 2014 10:03:14 -0500
changeset 163595 ea4cb98da6174f7351f3cd33d13cfaf26ba6c514
parent 163594 81e542a2ab7099e28d1971c137184f51c769c3c9
child 163596 a1a47d7530dcef913eb94a5e46b17970b4bc7777
push idunknown
push userunknown
push dateunknown
reviewersted, bsmedberg
bugs791784
milestone29.0a1
Bug 791784 - Add serialization to avoid a thread race when a plug-in crashes during Firefox shutdown. r=ted, r=bsmedberg
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -234,16 +234,20 @@ static const int kBreakpadReserveSizePar
 // this holds additional data sent via the API
 static Mutex* crashReporterAPILock;
 static Mutex* notesFieldLock;
 static AnnotationTable* crashReporterAPIData_Hash;
 static nsCString* crashReporterAPIData = nullptr;
 static nsCString* notesField = nullptr;
 static bool isGarbageCollecting;
 
+// Avoid a race during application termination.
+static Mutex* dumpSafetyLock;
+static bool isSafeToDump = false;
+
 // OOP crash reporting
 static CrashGenerationServer* crashServer; // chrome process has this
 
 #  if defined(XP_WIN) || defined(XP_MACOSX)
 // If crash reporting is disabled, we hand out this "null" pipe to the
 // child process and don't attempt to connect to a parent server.
 static const char kNullNotifyPipe[] = "-";
 static char* childCrashNotifyPipe;
@@ -995,16 +999,25 @@ nsresult SetExceptionHandler(nsIFile* aX
     minidump_type = MiniDumpWithFullMemory;
   }
 #endif // XP_WIN32
 
 #ifdef MOZ_WIDGET_ANDROID
   androidUserSerial = getenv("MOZ_ANDROID_USER_SERIAL_NUMBER");
 #endif
 
+  // Initialize the flag and mutex used to avoid dump processing
+  // once browser termination has begun.
+  NS_ASSERTION(!dumpSafetyLock, "Shouldn't have a lock yet");
+  // Do not deallocate this lock while it is still possible for
+  // isSafeToDump to be tested on another thread.
+  dumpSafetyLock = new Mutex("dumpSafetyLock");
+  MutexAutoLock lock(*dumpSafetyLock);
+  isSafeToDump = true;
+
   // now set the exception handler
 #ifdef XP_LINUX
   MinidumpDescriptor descriptor(tempPath.get());
 #endif
 
   gExceptionHandler = new google_breakpad::
     ExceptionHandler(
 #ifdef XP_LINUX
@@ -1340,16 +1353,21 @@ nsresult SetupExtraData(nsIFile* aAppDat
 
   return NS_OK;
 }
 
 static void OOPDeinit();
 
 nsresult UnsetExceptionHandler()
 {
+  if (isSafeToDump) {
+    MutexAutoLock lock(*dumpSafetyLock);
+    isSafeToDump = false;
+  }
+
 #ifdef XP_WIN
   // allow SetUnhandledExceptionFilter
   gBlockUnhandledExceptionFilter = false;
 #endif
 
   delete gExceptionHandler;
 
   // do this here in the unlikely case that we succeeded in allocating
@@ -1388,16 +1406,19 @@ nsresult UnsetExceptionHandler()
 
   if (!gExceptionHandler)
     return NS_ERROR_NOT_INITIALIZED;
 
   gExceptionHandler = nullptr;
 
   OOPDeinit();
 
+  delete dumpSafetyLock;
+  dumpSafetyLock = nullptr;
+
   return NS_OK;
 }
 
 static void ReplaceChar(nsCString& str, const nsACString& character,
                         const nsACString& replacement)
 {
   nsCString::const_iterator start, end;
 
@@ -2199,16 +2220,23 @@ OnChildProcessDumpRequested(void* aConte
                             const ClientInfo* aClientInfo,
                             const xpstring* aFilePath
 #endif
                             )
 {
   nsCOMPtr<nsIFile> minidump;
   nsCOMPtr<nsIFile> extraFile;
 
+  // Hold the mutex until the current dump request is complete, to
+  // prevent UnsetExceptionHandler() from pulling the rug out from
+  // under us.
+  MutexAutoLock lock(*dumpSafetyLock);
+  if (!isSafeToDump)
+    return;
+
   CreateFileFromPath(
 #ifdef XP_MACOSX
                      aFilePath,
 #else
                      *aFilePath,
 #endif
                      getter_AddRefs(minidump));