Bug 1250637: Allow child process crash annotation off main thread. r=aklotz draft
authorMilan Sreckovic <milan@mozilla.com>
Tue, 10 May 2016 15:59:00 -0400
changeset 365461 a42e0576de0ede41f522825cacdd8ce16313c953
parent 365273 1579b9e2e50f3a27ad02d58cc9170c91e0973fec
child 520561 c488a7a1ba13b0292bc6561af191d09bdbfd4568
push id17745
push usermsreckovic@mozilla.com
push dateTue, 10 May 2016 19:59:13 +0000
reviewersaklotz
bugs1250637
milestone49.0a1
Bug 1250637: Allow child process crash annotation off main thread. r=aklotz MozReview-Commit-ID: B1oq7KlZrff
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -2106,31 +2106,58 @@ static void
 EnqueueDelayedNote(DelayedNote* aNote)
 {
   if (!gDelayedAnnotations) {
     gDelayedAnnotations = new nsTArray<nsAutoPtr<DelayedNote> >();
   }
   gDelayedAnnotations->AppendElement(aNote);
 }
 
+class CrashReporterHelperRunnable : public Runnable {
+public:
+  explicit CrashReporterHelperRunnable(const nsACString& aKey,
+                                       const nsACString& aData)
+    : mKey(aKey)
+    , mData(aData)
+    , mAppendAppNotes(false)
+    {}
+  explicit CrashReporterHelperRunnable(const nsACString& aData)
+    : mKey()
+    , mData(aData)
+    , mAppendAppNotes(true)
+    {}
+
+  NS_METHOD Run() override;
+
+private:
+  nsCString mKey;
+  nsCString mData;
+  bool mAppendAppNotes;
+};
+
 nsresult AnnotateCrashReport(const nsACString& key, const nsACString& data)
 {
   if (!GetEnabled())
     return NS_ERROR_NOT_INITIALIZED;
 
+  bool isParentProcess = XRE_IsParentProcess();
+  if (!isParentProcess && !NS_IsMainThread()) {
+    // Child process needs to handle this in the main thread:
+    nsCOMPtr<nsIRunnable> r = new CrashReporterHelperRunnable(key, data);
+    NS_DispatchToMainThread(r);
+    return NS_OK;
+  }
+
   nsCString escapedData;
   nsresult rv = EscapeAnnotation(key, data, escapedData);
   if (NS_FAILED(rv))
     return rv;
 
-  if (!XRE_IsParentProcess()) {
-    if (!NS_IsMainThread()) {
-      NS_ERROR("Cannot call AnnotateCrashReport in child processes from non-main thread.");
-      return NS_ERROR_FAILURE;
-    }
+  if (!isParentProcess) {
+    MOZ_ASSERT(NS_IsMainThread());
     PCrashReporterChild* reporter = CrashReporterChild::GetCrashReporter();
     if (!reporter) {
       EnqueueDelayedNote(new DelayedNote(key, data));
       return NS_OK;
     }
     if (!reporter->SendAnnotateCrashReport(nsCString(key), escapedData))
       return NS_ERROR_FAILURE;
     return NS_OK;
@@ -2184,21 +2211,26 @@ void SetEventloopNestingLevel(uint32_t l
 nsresult AppendAppNotesToCrashReport(const nsACString& data)
 {
   if (!GetEnabled())
     return NS_ERROR_NOT_INITIALIZED;
 
   if (DoFindInReadable(data, NS_LITERAL_CSTRING("\0")))
     return NS_ERROR_INVALID_ARG;
 
+  bool isParentProcess = XRE_IsParentProcess();
+  if (!isParentProcess && !NS_IsMainThread()) {
+    // Child process needs to handle this in the main thread:
+    nsCOMPtr<nsIRunnable> r = new CrashReporterHelperRunnable(data);
+    NS_DispatchToMainThread(r);
+    return NS_OK;
+  }
+
   if (!XRE_IsParentProcess()) {
-    if (!NS_IsMainThread()) {
-      NS_ERROR("Cannot call AnnotateCrashReport in child processes from non-main thread.");
-      return NS_ERROR_FAILURE;
-    }
+    MOZ_ASSERT(NS_IsMainThread());
     PCrashReporterChild* reporter = CrashReporterChild::GetCrashReporter();
     if (!reporter) {
       EnqueueDelayedNote(new DelayedNote(data));
       return NS_OK;
     }
 
     // Since we don't go through AnnotateCrashReport in the parent process,
     // we must ensure that the data is escaped and valid before the parent
@@ -2214,16 +2246,34 @@ nsresult AppendAppNotesToCrashReport(con
   }
 
   MutexAutoLock lock(*notesFieldLock);
 
   notesField->Append(data);
   return AnnotateCrashReport(NS_LITERAL_CSTRING("Notes"), *notesField);
 }
 
+nsresult CrashReporterHelperRunnable::Run()
+{
+  // We expect this to be in the child process' main thread.  If it isn't,
+  // something is happening we didn't design for.
+  MOZ_ASSERT(!XRE_IsParentProcess());
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Don't just leave the assert, paranoid about infinite recursion
+  if (NS_IsMainThread()) {
+    if (mAppendAppNotes) {
+      return AppendAppNotesToCrashReport(mData);
+    } else {
+      return AnnotateCrashReport(mKey, mData);
+    }
+  }
+  return NS_ERROR_FAILURE;
+}
+
 // Returns true if found, false if not found.
 bool GetAnnotation(const nsACString& key, nsACString& data)
 {
   if (!gExceptionHandler)
     return false;
 
   nsAutoCString entry;
   if (!crashReporterAPIData_Hash->Get(key, &entry))