Bug 1408806 - Gracefully deal with errors when killing the minidump-analyzer on shutdown; r?mconley draft
authorGabriele Svelto <gsvelto@mozilla.com>
Thu, 26 Oct 2017 22:59:29 +0200
changeset 687065 3e764cf63159afdbea3d4705d603d97b1d668707
parent 686335 a97fb1c39d51a7337b46957bde4273bd308b796a
child 737569 cea73426756281e212e25ad5cc05bb2bfc046ba1
push id86407
push usergsvelto@mozilla.com
push dateThu, 26 Oct 2017 21:03:13 +0000
reviewersmconley
bugs1408806
milestone58.0a1
Bug 1408806 - Gracefully deal with errors when killing the minidump-analyzer on shutdown; r?mconley This solves two problems that were causing tests to fail intermittently. The first issue is that calling nsIProcess.kill() on a process that had already terminated would throw an exception which wouldn't be caught. Since might happen in cases where the minidump-analyzer run significantly faster than the main event loop during the test. The second issue is that we tried to unconditionally escape both the 'TelemetryEnvironment' and 'StackTraces' field, but the latter would not be present in cases where the minidump-analyzer would fail or be killed. This led to another spurious exception. MozReview-Commit-ID: 7srQtzig7xw
toolkit/components/crashes/CrashService.js
--- a/toolkit/components/crashes/CrashService.js
+++ b/toolkit/components/crashes/CrashService.js
@@ -120,17 +120,19 @@ function processExtraFile(extraPath) {
       let extraData = await OS.File.read(extraPath);
       let keyValuePairs = parseKeyValuePairs(decoder.decode(extraData));
 
       // When reading from an .extra file literal '\\n' sequences are
       // automatically unescaped to two backslashes plus a newline, so we need
       // to re-escape them into '\\n' again so that the fields holding JSON
       // strings are valid.
       [ "TelemetryEnvironment", "StackTraces" ].forEach(field => {
-        keyValuePairs[field] = keyValuePairs[field].replace(/\n/g, "n");
+        if (field in keyValuePairs) {
+          keyValuePairs[field] = keyValuePairs[field].replace(/\n/g, "n");
+        }
       });
 
       return keyValuePairs;
     } catch (e) {
       Cu.reportError(e);
       return {};
     }
   })();
@@ -220,17 +222,22 @@ CrashService.prototype = Object.freeze({
     switch (topic) {
       case "profile-after-change":
         // Side-effect is the singleton is instantiated.
         Services.crashmanager;
         break;
       case "quit-application":
         gQuitting = true;
         gRunningProcesses.forEach((process) => {
-          process.kill();
+          try {
+            process.kill();
+          } catch (e) {
+            // If the process has already quit then kill() fails, but since
+            // this failure is benign it is safe to silently ignore it.
+          }
           Services.obs.notifyObservers(null, "test-minidump-analyzer-killed");
         });
         break;
     }
   },
 });
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([CrashService]);