Backed out changeset 1601c1a2cbaf (bug 917883) for frequent shutdown crashes.
authorRyan VanderMeulen <ryanvm@gmail.com>
Tue, 29 Apr 2014 20:34:23 -0400
changeset 181314 5b99fe773ae9e66089e93b03299a67e75a2ccb7e
parent 181313 253acf38522981623e9c02d5e57ae8c48794ed51
child 181315 6eed9c67a1e37b1681c94c3234ce25343d6abbae
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
bugs917883
milestone32.0a1
backs out1601c1a2cbafbfe43533067a61ce9913ff527315
Backed out changeset 1601c1a2cbaf (bug 917883) for frequent shutdown crashes.
services/healthreport/healthreporter.jsm
services/healthreport/tests/xpcshell/test_healthreporter.js
services/healthreport/tests/xpcshell/test_provider_appinfo.js
services/metrics/storage.jsm
toolkit/components/telemetry/Histograms.json
--- a/services/healthreport/healthreporter.jsm
+++ b/services/healthreport/healthreporter.jsm
@@ -25,18 +25,16 @@ Cu.import("resource://gre/modules/osfile
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel",
                                   "resource://gre/modules/UpdateChannel.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
-                                  "resource://gre/modules/AsyncShutdown.jsm");
 
 // Oldest year to allow in date preferences. This module was implemented in
 // 2012 and no dates older than that should be encountered.
 const OLDEST_ALLOWED_YEAR = 2012;
 
 const DAYS_IN_PAYLOAD = 180;
 
 const DEFAULT_DATABASE_NAME = "healthreport.sqlite";
@@ -45,16 +43,17 @@ const TELEMETRY_INIT = "HEALTHREPORT_INI
 const TELEMETRY_INIT_FIRSTRUN = "HEALTHREPORT_INIT_FIRSTRUN_MS";
 const TELEMETRY_DB_OPEN = "HEALTHREPORT_DB_OPEN_MS";
 const TELEMETRY_DB_OPEN_FIRSTRUN = "HEALTHREPORT_DB_OPEN_FIRSTRUN_MS";
 const TELEMETRY_GENERATE_PAYLOAD = "HEALTHREPORT_GENERATE_JSON_PAYLOAD_MS";
 const TELEMETRY_JSON_PAYLOAD_SERIALIZE = "HEALTHREPORT_JSON_PAYLOAD_SERIALIZE_MS";
 const TELEMETRY_PAYLOAD_SIZE_UNCOMPRESSED = "HEALTHREPORT_PAYLOAD_UNCOMPRESSED_BYTES";
 const TELEMETRY_PAYLOAD_SIZE_COMPRESSED = "HEALTHREPORT_PAYLOAD_COMPRESSED_BYTES";
 const TELEMETRY_UPLOAD = "HEALTHREPORT_UPLOAD_MS";
+const TELEMETRY_SHUTDOWN_DELAY = "HEALTHREPORT_SHUTDOWN_DELAY_MS";
 const TELEMETRY_COLLECT_CONSTANT = "HEALTHREPORT_COLLECT_CONSTANT_DATA_MS";
 const TELEMETRY_COLLECT_DAILY = "HEALTHREPORT_COLLECT_DAILY_MS";
 const TELEMETRY_SHUTDOWN = "HEALTHREPORT_SHUTDOWN_MS";
 const TELEMETRY_COLLECT_CHECKPOINT = "HEALTHREPORT_POST_COLLECT_CHECKPOINT_MS";
 
 
 /**
  * Helper type to assist with management of Health Reporter state.
@@ -125,17 +124,23 @@ HealthReporterState.prototype = Object.f
   },
 
   get _lastPayloadPath() {
     return OS.Path.join(this._stateDir, "lastpayload.json");
   },
 
   init: function () {
     return Task.spawn(function init() {
-      OS.File.makeDir(this._stateDir);
+      try {
+        OS.File.makeDir(this._stateDir);
+      } catch (ex if ex instanceof OS.FileError) {
+        if (!ex.becauseExists) {
+          throw ex;
+        }
+      }
 
       let resetObjectState = function () {
         this._s = {
           // The payload version. This is bumped whenever there is a
           // backwards-incompatible change.
           v: 1,
           // The persistent client identifier.
           clientID: CommonUtils.generateUUID(),
@@ -148,18 +153,21 @@ HealthReporterState.prototype = Object.f
           lastPingTime: 0,
           // Tracks whether we removed an outdated payload.
           removedOutdatedLastpayload: false,
         };
       }.bind(this);
 
       try {
         this._s = yield CommonUtils.readJSON(this._filename);
-      } catch (ex if ex instanceof OS.File.Error &&
-               ex.becauseNoSuchFile) {
+      } catch (ex if ex instanceof OS.File.Error) {
+        if (!ex.becauseNoSuchFile) {
+          throw ex;
+        }
+
         this._log.warn("Saved state file does not exist.");
         resetObjectState();
       } catch (ex) {
         this._log.error("Exception when reading state from disk: " +
                         CommonUtils.exceptionStr(ex));
         resetObjectState();
 
         // Don't save in case it goes away on next run.
@@ -336,18 +344,17 @@ function AbstractHealthReporter(branch, 
   this._providerManagerInProgress = false;
   this._initializeStarted = false;
   this._initialized = false;
   this._initializeHadError = false;
   this._initializedDeferred = Promise.defer();
   this._shutdownRequested = false;
   this._shutdownInitiated = false;
   this._shutdownComplete = false;
-  this._deferredShutdown = Promise.defer();
-  this._promiseShutdown = this._deferredShutdown.promise;
+  this._shutdownCompleteCallback = null;
 
   this._errors = [];
 
   this._lastDailyDate = null;
 
   // Yes, this will probably run concurrently with remaining constructor work.
   let hasFirstRun = this._prefs.get("service.firstRun", false);
   this._initHistogram = hasFirstRun ? TELEMETRY_INIT : TELEMETRY_INIT_FIRSTRUN;
@@ -374,69 +381,22 @@ AbstractHealthReporter.prototype = Objec
    */
   init: function () {
     if (this._initializeStarted) {
       throw new Error("We have already started initialization.");
     }
 
     this._initializeStarted = true;
 
-    return Task.spawn(function*() {
-      TelemetryStopwatch.start(this._initHistogram, this);
-
-      try {
-        yield this._state.init();
-
-        if (!this._state._s.removedOutdatedLastpayload) {
-          yield this._deleteOldLastPayload();
-          this._state._s.removedOutdatedLastpayload = true;
-          // Normally we should save this to a file but it directly conflicts with
-          // the "application re-upgrade" decision in HealthReporterState::init()
-          // which specifically does not save the state to a file.
-        }
-      } catch (ex) {
-        this._log.error("Error deleting last payload: " +
-                        CommonUtils.exceptionStr(ex));
-      }
-
-      // As soon as we have could have storage, we need to register cleanup or
-      // else bad things happen on shutdown.
-      Services.obs.addObserver(this, "quit-application", false);
-
-      // The database needs to be shut down by the end of shutdown
-      // phase profileBeforeChange.
-      AsyncShutdown.profileBeforeChange.
-        addBlocker("FHR: Flushing storage shutdown", this._promiseShutdown);
+    TelemetryStopwatch.start(this._initHistogram, this);
 
-      try {
-        this._storageInProgress = true;
-        TelemetryStopwatch.start(this._dbOpenHistogram, this);
-        let storage = yield Metrics.Storage(this._dbName);
-        TelemetryStopwatch.finish(this._dbOpenHistogram, this);
-        yield this._onStorageCreated();
-
-        delete this._dbOpenHistogram;
-        this._log.info("Storage initialized.");
-        this._storage = storage;
-        this._storageInProgress = false;
+    this._initializeState().then(this._onStateInitialized.bind(this),
+                                 this._onInitError.bind(this));
 
-        if (this._shutdownRequested) {
-          this._initiateShutdown();
-          return null;
-        }
-
-        yield this._initializeProviderManager();
-        yield this._onProviderManagerInitialized();
-        this._initializedDeferred.resolve();
-        return this.onInit();
-      } catch (ex) {
-        yield this._onInitError(ex);
-        this._initializedDeferred.reject(ex);
-      }
-    }.bind(this));
+    return this.onInit();
   },
 
   //----------------------------------------------------
   // SERVICE CONTROL FUNCTIONS
   //
   // You shouldn't need to call any of these externally.
   //----------------------------------------------------
 
@@ -444,22 +404,53 @@ AbstractHealthReporter.prototype = Objec
     TelemetryStopwatch.cancel(this._initHistogram, this);
     TelemetryStopwatch.cancel(this._dbOpenHistogram, this);
     delete this._initHistogram;
     delete this._dbOpenHistogram;
 
     this._recordError("Error during initialization", error);
     this._initializeHadError = true;
     this._initiateShutdown();
-    return Promise.reject(error);
+    this._initializedDeferred.reject(error);
 
     // FUTURE consider poisoning prototype's functions so calls fail with a
     // useful error message.
   },
 
+  _initializeState: function () {
+    return Promise.resolve();
+  },
+
+  _onStateInitialized: function () {
+    return Task.spawn(function onStateInitialized () {
+      try {
+        if (!this._state._s.removedOutdatedLastpayload) {
+          yield this._deleteOldLastPayload();
+          this._state._s.removedOutdatedLastpayload = true;
+          // Normally we should save this to a file but it directly conflicts with
+          //  the "application re-upgrade" decision in HealthReporterState::init()
+          //  which specifically does not save the state to a file.
+        }
+      } catch (ex) {
+        this._log.error("Error deleting last payload: " +
+                        CommonUtils.exceptionStr(ex));
+      }
+      // As soon as we have could storage, we need to register cleanup or
+      // else bad things happen on shutdown.
+      Services.obs.addObserver(this, "quit-application", false);
+      Services.obs.addObserver(this, "profile-before-change", false);
+
+      this._storageInProgress = true;
+      TelemetryStopwatch.start(this._dbOpenHistogram, this);
+
+      Metrics.Storage(this._dbName).then(this._onStorageCreated.bind(this),
+                                         this._onInitError.bind(this));
+    }.bind(this));
+  },
+
 
   /**
    * Removes the outdated lastpaylaod.json and lastpayload.json.tmp files
    * @see Bug #867902
    * @return a promise for when all the files have been deleted
    */
   _deleteOldLastPayload: function () {
     let paths = [this._state._lastPayloadPath, this._state._lastPayloadPath + ".tmp"];
@@ -472,16 +463,34 @@ AbstractHealthReporter.prototype = Objec
             this._log.error("Exception when removing outdated payload files: " +
                             CommonUtils.exceptionStr(ex));
           }
         }
       }
     }.bind(this));
   },
 
+  // Called when storage has been opened.
+  _onStorageCreated: function (storage) {
+    TelemetryStopwatch.finish(this._dbOpenHistogram, this);
+    delete this._dbOpenHistogram;
+    this._log.info("Storage initialized.");
+    this._storage = storage;
+    this._storageInProgress = false;
+
+    if (this._shutdownRequested) {
+      this._initiateShutdown();
+      return;
+    }
+
+    Task.spawn(this._initializeProviderManager.bind(this))
+        .then(this._onProviderManagerInitialized.bind(this),
+              this._onInitError.bind(this));
+  },
+
   _initializeProviderManager: function () {
     if (this._collector) {
       throw new Error("Provider manager has already been initialized.");
     }
 
     this._log.info("Initializing provider manager.");
     this._providerManager = new Metrics.ProviderManager(this._storage);
     this._providerManager.onProviderError = this._recordError.bind(this);
@@ -537,26 +546,32 @@ AbstractHealthReporter.prototype = Objec
       } catch (ex) {
         this._log.error("Error registering collection timer: " +
                         CommonUtils.exceptionStr(ex));
       }
     }
 
     // Clean up caches and reduce memory usage.
     this._storage.compact();
+    this._initializedDeferred.resolve(this);
   },
 
   // nsIObserver to handle shutdown.
   observe: function (subject, topic, data) {
     switch (topic) {
       case "quit-application":
         Services.obs.removeObserver(this, "quit-application");
         this._initiateShutdown();
         break;
 
+      case "profile-before-change":
+        Services.obs.removeObserver(this, "profile-before-change");
+        this._waitForShutdown();
+        break;
+
       case "idle-daily":
         this._performDailyMaintenance();
         break;
     }
   },
 
   _initiateShutdown: function () {
     // Ensure we only begin the main shutdown sequence once.
@@ -597,80 +612,115 @@ AbstractHealthReporter.prototype = Objec
     this._shutdownInitiated = true;
 
     // We may not have registered the observer yet. If not, this will
     // throw.
     try {
       Services.obs.removeObserver(this, "idle-daily");
     } catch (ex) { }
 
-    Task.spawn(function*() {
+    if (this._providerManager) {
+      let onShutdown = this._onProviderManagerShutdown.bind(this);
+      Task.spawn(this._shutdownProviderManager.bind(this))
+          .then(onShutdown, onShutdown);
+      return;
+    }
+
+    this._log.warn("Don't have provider manager. Proceeding to storage shutdown.");
+    this._shutdownStorage();
+  },
+
+  _shutdownProviderManager: function () {
+    this._log.info("Shutting down provider manager.");
+    for (let provider of this._providerManager.providers) {
       try {
-        if (this._providerManager) {
-          this._log.info("Shutting down provider manager.");
-          for (let provider of this._providerManager.providers) {
-            try {
-              yield provider.shutdown();
-            } catch (ex) {
-              this._log.warn("Error when shutting down provider: " +
-                             CommonUtils.exceptionStr(ex));
-            }
-          }
-          this._log.info("Provider manager shut down.");
-          this._providerManager = null;
-          this._onProviderManagerShutdown();
-        }
-        if (this._storage) {
-          this._log.info("Shutting down storage.");
-          try {
-            yield this._storage.close();
-            yield this._onStorageClose();
-          } catch (error) {
-            this._log.warn("Error when closing storage: " +
-                           CommonUtils.exceptionStr(error));
-          }
-          this._storage = null;
-        }
+        yield provider.shutdown();
+      } catch (ex) {
+        this._log.warn("Error when shutting down provider: " +
+                       CommonUtils.exceptionStr(ex));
+      }
+    }
+  },
 
-        this._log.warn("Shutdown complete.");
-        this._shutdownComplete = true;
-      } finally {
-        this._deferredShutdown.resolve();
-        TelemetryStopwatch.finish(TELEMETRY_SHUTDOWN, this);
-      }
-    }.bind(this));
+  _onProviderManagerShutdown: function () {
+    this._log.info("Provider manager shut down.");
+    this._providerManager = null;
+    this._shutdownStorage();
   },
 
-  onInit: function() {
-    return this._initializedDeferred.promise;
+  _shutdownStorage: function () {
+    if (!this._storage) {
+      this._onShutdownComplete();
+    }
+
+    this._log.info("Shutting down storage.");
+    let onClose = this._onStorageClose.bind(this);
+    this._storage.close().then(onClose, onClose);
   },
 
-  _onStorageCreated: function() {
-    // Do nothing.
-    // This method provides a hook point for the test suite.
+  _onStorageClose: function (error) {
+    this._log.info("Storage has been closed.");
+
+    if (error) {
+      this._log.warn("Error when closing storage: " +
+                     CommonUtils.exceptionStr(error));
+    }
+
+    this._storage = null;
+    this._onShutdownComplete();
   },
 
-  _onStorageClose: function() {
-    // Do nothing.
-    // This method provides a hook point for the test suite.
+  _onShutdownComplete: function () {
+    this._log.warn("Shutdown complete.");
+    this._shutdownComplete = true;
+    TelemetryStopwatch.finish(TELEMETRY_SHUTDOWN, this);
+
+    if (this._shutdownCompleteCallback) {
+      this._shutdownCompleteCallback();
+    }
   },
 
-  _onProviderManagerShutdown: function() {
-    // Do nothing.
-    // This method provides a hook point for the test suite.
+  _waitForShutdown: function () {
+    if (this._shutdownComplete) {
+      return;
+    }
+
+    TelemetryStopwatch.start(TELEMETRY_SHUTDOWN_DELAY, this);
+    try {
+      this._shutdownCompleteCallback = Async.makeSpinningCallback();
+      this._shutdownCompleteCallback.wait();
+      this._shutdownCompleteCallback = null;
+    } finally {
+      TelemetryStopwatch.finish(TELEMETRY_SHUTDOWN_DELAY, this);
+    }
   },
 
   /**
    * Convenience method to shut down the instance.
    *
    * This should *not* be called outside of tests.
    */
   _shutdown: function () {
     this._initiateShutdown();
-    return this._promiseShutdown;
+    this._waitForShutdown();
+  },
+
+  /**
+   * Return a promise that is resolved once the service has been initialized.
+   */
+  onInit: function () {
+    if (this._initializeHadError) {
+      throw new Error("Service failed to initialize.");
+    }
+
+    if (this._initialized) {
+      return CommonUtils.laterTickResolvingPromise(this);
+    }
+
+    return this._initializedDeferred.promise;
   },
 
   _performDailyMaintenance: function () {
     this._log.info("Request to perform daily maintenance.");
 
     if (!this._initialized) {
       return;
     }
@@ -1292,16 +1342,20 @@ this.HealthReporter.prototype = Object.f
   requestDeleteRemoteData: function (reason) {
     if (!this.haveRemoteData()) {
       return;
     }
 
     return this._policy.deleteRemoteData(reason);
   },
 
+  _initializeState: function() {
+    return this._state.init();
+  },
+
   /**
    * Override default handler to incur an upload describing the error.
    */
   _onInitError: function (error) {
     // Need to capture this before we call the parent else it's always
     // set.
     let inShutdown = this._shutdownRequested;
 
--- a/services/healthreport/tests/xpcshell/test_healthreporter.js
+++ b/services/healthreport/tests/xpcshell/test_healthreporter.js
@@ -98,16 +98,17 @@ function run_test() {
 
 // run_test() needs to finish synchronously, so we do async init here.
 add_task(function test_init() {
   yield makeFakeAppDir();
 });
 
 add_task(function test_constructor() {
   let reporter = yield getReporter("constructor");
+
   try {
     do_check_eq(reporter.lastPingDate.getTime(), 0);
     do_check_null(reporter.lastSubmitID);
     do_check_eq(typeof(reporter._state), "object");
     do_check_eq(reporter._state.lastPingDate.getTime(), 0);
     do_check_eq(reporter._state.remoteIDs.length, 0);
     do_check_eq(reporter._state.clientIDVersion, 1);
     do_check_neq(reporter._state.clientID, null);
@@ -118,40 +119,40 @@ add_task(function test_constructor() {
     } catch (ex) {
       failed = true;
       do_check_true(ex.message.startsWith("Branch must end"));
     } finally {
       do_check_true(failed);
       failed = false;
     }
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_shutdown_normal() {
   let reporter = yield getReporter("shutdown_normal");
 
   // We can't send "quit-application" notification because the xpcshell runner
   // will shut down!
   reporter._initiateShutdown();
-  yield reporter._promiseShutdown;
+  reporter._waitForShutdown();
 });
 
 add_task(function test_shutdown_storage_in_progress() {
   let reporter = yield getHealthReporter("shutdown_storage_in_progress", DUMMY_URI, true);
 
   reporter.onStorageCreated = function () {
     print("Faking shutdown during storage initialization.");
     reporter._initiateShutdown();
   };
 
   reporter.init();
 
-  yield reporter._promiseShutdown;
+  reporter._waitForShutdown();
   do_check_eq(reporter.providerManagerShutdownCount, 0);
   do_check_eq(reporter.storageCloseCount, 1);
 });
 
 // Ensure that a shutdown triggered while provider manager is initializing
 // results in shutdown and storage closure.
 add_task(function test_shutdown_provider_manager_in_progress() {
   let reporter = yield getHealthReporter("shutdown_provider_manager_in_progress",
@@ -160,17 +161,17 @@ add_task(function test_shutdown_provider
   reporter.onProviderManagerInitialized = function () {
     print("Faking shutdown during provider manager initialization.");
     reporter._initiateShutdown();
   };
 
   reporter.init();
 
   // This will hang if shutdown logic is busted.
-  yield reporter._promiseShutdown;
+  reporter._waitForShutdown();
   do_check_eq(reporter.providerManagerShutdownCount, 1);
   do_check_eq(reporter.storageCloseCount, 1);
 });
 
 // Simulates an error during provider manager initialization and verifies we shut down.
 add_task(function test_shutdown_when_provider_manager_errors() {
   let reporter = yield getHealthReporter("shutdown_when_provider_manager_errors",
                                        DUMMY_URI, true);
@@ -178,17 +179,17 @@ add_task(function test_shutdown_when_pro
   reporter.onInitializeProviderManagerFinished = function () {
     print("Throwing fake error.");
     throw new Error("Fake error during provider manager initialization.");
   };
 
   reporter.init();
 
   // This will hang if shutdown logic is busted.
-  yield reporter._promiseShutdown;
+  reporter._waitForShutdown();
   do_check_eq(reporter.providerManagerShutdownCount, 1);
   do_check_eq(reporter.storageCloseCount, 1);
 });
 
 // Pull-only providers are only initialized at collect time.
 add_task(function test_pull_only_providers() {
   const category = "healthreporter-constant-only";
 
@@ -215,17 +216,17 @@ add_task(function test_pull_only_provide
 
     do_check_eq(reporter._providerManager._providers.size, initCount + 1);
     do_check_true(reporter._storage.hasProvider("DummyConstantProvider"));
 
     let mID = reporter._storage.measurementID("DummyConstantProvider", "DummyMeasurement", 1);
     let values = yield reporter._storage.getMeasurementValues(mID);
     do_check_true(values.singular.size > 0);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_collect_daily() {
   let reporter = yield getReporter("collect_daily");
 
   try {
     let now = new Date();
@@ -246,17 +247,17 @@ add_task(function test_collect_daily() {
     reporter._lastDailyDate = now.getTime() - MILLISECONDS_PER_DAY - 1;
     yield reporter.collectMeasurements();
     do_check_eq(provider.collectDailyCount, 2);
 
     reporter._lastDailyDate = null;
     yield reporter.collectMeasurements();
     do_check_eq(provider.collectDailyCount, 3);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_remove_old_lastpayload() {
   let reporter = getHealthReporter("remove-old-lastpayload");
   let lastPayloadPath = reporter._state._lastPayloadPath;
   let paths = [lastPayloadPath, lastPayloadPath + ".tmp"];
   let createFiles = function () {
@@ -270,29 +271,29 @@ add_task(function test_remove_old_lastpa
   try {
     do_check_true(!reporter._state.removedOutdatedLastpayload);
     yield createFiles();
     yield reporter.init();
     for (let path of paths) {
       do_check_false(yield OS.File.exists(path));
     }
     yield reporter._state.save();
-    yield reporter._shutdown();
+    reporter._shutdown();
 
     let o = yield CommonUtils.readJSON(reporter._state._filename);
     do_check_true(o.removedOutdatedLastpayload);
 
     yield createFiles();
     reporter = getHealthReporter("remove-old-lastpayload");
     yield reporter.init();
     for (let path of paths) {
       do_check_true(yield OS.File.exists(path));
     }
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_json_payload_simple() {
   let reporter = yield getReporter("json_payload_simple");
 
   let clientID = reporter._state.clientID;
   do_check_neq(clientID, null);
@@ -321,17 +322,17 @@ add_task(function test_json_payload_simp
 
     // This could fail if we cross UTC day boundaries at the exact instance the
     // test is executed. Let's tempt fate.
     do_check_eq(original.thisPingDate, reporter._formatDate(now));
 
     payload = yield reporter.getJSONPayload(true);
     do_check_eq(typeof payload, "object");
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_json_payload_dummy_provider() {
   let reporter = yield getReporter("json_payload_dummy_provider");
 
   try {
     yield reporter._providerManager.registerProvider(new DummyProvider());
@@ -340,17 +341,17 @@ add_task(function test_json_payload_dumm
     print(payload);
     let o = JSON.parse(payload);
 
     let name = "DummyProvider.DummyMeasurement";
     do_check_eq(Object.keys(o.data.last).length, 1);
     do_check_true(name in o.data.last);
     do_check_eq(o.data.last[name]._v, 1);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_collect_and_obtain_json_payload() {
   let reporter = yield getReporter("collect_and_obtain_json_payload");
 
   try {
     yield reporter._providerManager.registerProvider(new DummyProvider());
@@ -358,17 +359,17 @@ add_task(function test_collect_and_obtai
     do_check_eq(typeof payload, "string");
 
     let o = JSON.parse(payload);
     do_check_true("DummyProvider.DummyMeasurement" in o.data.last);
 
     payload = yield reporter.collectAndObtainJSONPayload(true);
     do_check_eq(typeof payload, "object");
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // Ensure constant-only providers make their way into the JSON payload.
 add_task(function test_constant_only_providers_in_json_payload() {
   const category = "healthreporter-constant-only-in-payload";
 
   let cm = Cc["@mozilla.org/categorymanager;1"]
@@ -414,17 +415,17 @@ add_task(function test_constant_only_pro
     reporter.collectAndObtainJSONPayload().then(do_throw, function onError() {
       providers = reporter._providerManager.providers;
       do_check_eq(providers.length, initCount + 1);
       deferred.resolve();
     });
 
     yield deferred.promise;
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_json_payload_multiple_days() {
   let reporter = yield getReporter("json_payload_multiple_days");
 
   try {
     let provider = new DummyProvider();
@@ -449,17 +450,17 @@ add_task(function test_json_payload_mult
     let payload = yield reporter.getJSONPayload();
     print(payload);
     let o = JSON.parse(payload);
 
     do_check_eq(Object.keys(o.data.days).length, 180);
     let today = reporter._formatDate(now);
     do_check_true(today in o.data.days);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_json_payload_newer_version_overwrites() {
   let reporter = yield getReporter("json_payload_newer_version_overwrites");
 
   try {
     let now = new Date();
@@ -526,17 +527,17 @@ add_task(function test_json_payload_newe
     do_check_eq(o.data.last["MultiMeasurementProvider.DummyMeasurement"]._v, highestVersion);
 
     let day = reporter._formatDate(now);
     do_check_true(day in o.data.days);
     do_check_true("MultiMeasurementProvider.DummyMeasurement" in o.data.days[day]);
     do_check_eq(o.data.days[day]["MultiMeasurementProvider.DummyMeasurement"]._v, highestVersion);
 
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_idle_daily() {
   let reporter = yield getReporter("idle_daily");
   try {
     let provider = new DummyProvider();
     yield reporter._providerManager.registerProvider(provider);
@@ -551,17 +552,17 @@ add_task(function test_idle_daily() {
     let values = yield m.getValues();
     do_check_eq(values.days.size, 200);
 
     Services.obs.notifyObservers(null, "idle-daily", null);
 
     values = yield m.getValues();
     do_check_eq(values.days.size, 180);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_data_submission_transport_failure() {
   let reporter = yield getReporter("data_submission_transport_failure");
   try {
     reporter.serverURI = DUMMY_URI;
     reporter.serverNamespace = "test00";
@@ -641,25 +642,25 @@ add_task(function test_data_submission_s
     do_check_eq(data.continuationUploadAttempt, 1);
     do_check_eq(data.uploadSuccess, 1);
     do_check_eq(Object.keys(data).length, 3);
 
     let d = reporter.lastPingDate;
     let id = reporter.lastSubmitID;
     let clientID = reporter._state.clientID;
 
-    yield reporter._shutdown();
+    reporter._shutdown();
 
     // Ensure reloading state works.
     reporter = yield getReporter("data_submission_success");
     do_check_eq(reporter.lastSubmitID, id);
     do_check_eq(reporter.lastPingDate.getTime(), d.getTime());
     do_check_eq(reporter._state.clientID, clientID);
 
-    yield reporter._shutdown();
+    reporter._shutdown();
   } finally {
     yield shutdownServer(server);
   }
 });
 
 add_task(function test_recurring_daily_pings() {
   let [reporter, server] = yield getReporterAndServer("recurring_daily_pings");
   try {
@@ -800,17 +801,17 @@ add_task(function test_policy_accept_rej
     policy.recordUserAcceptance();
     do_check_true(policy.dataSubmissionPolicyAccepted);
     do_check_true(reporter.willUploadData);
 
     policy.recordUserRejection();
     do_check_false(policy.dataSubmissionPolicyAccepted);
     do_check_false(reporter.willUploadData);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
     yield shutdownServer(server);
   }
 });
 
 add_task(function test_error_message_scrubbing() {
   let reporter = yield getReporter("error_message_scrubbing");
 
   try {
@@ -823,17 +824,17 @@ add_task(function test_error_message_scr
     reporter._errors = [];
 
     let appdata = Services.dirsvc.get("UAppData", Ci.nsIFile);
     let uri = Services.io.newFileURI(appdata);
 
     reporter._recordError("Foo " + uri.spec);
     do_check_eq(reporter._errors[0], "Foo <AppDataURI>");
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_basic_appinfo() {
   function verify(d) {
     do_check_eq(d["_v"], 1);
     do_check_eq(d._v, 1);
     do_check_eq(d.vendor, "Mozilla");
@@ -849,17 +850,17 @@ add_task(function test_basic_appinfo() {
   }
   let reporter = yield getReporter("basic_appinfo");
   try {
     verify(reporter.obtainAppInfo());
     let payload = yield reporter.collectAndObtainJSONPayload(true);
     do_check_eq(payload["version"], 2);
     verify(payload["geckoAppInfo"]);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // Ensure collection occurs if upload is disabled.
 add_task(function test_collect_when_upload_disabled() {
   let reporter = getHealthReporter("collect_when_upload_disabled");
   reporter._policy.recordHealthReportUploadEnabled(false, "testing-collect");
   do_check_false(reporter._policy.healthReportUploadEnabled);
@@ -870,23 +871,23 @@ add_task(function test_collect_when_uplo
 
   try {
     yield reporter.init();
     do_check_true(Services.prefs.prefHasUserValue(pref));
 
     // We would ideally ensure the timer fires and does the right thing.
     // However, testing the update timer manager is quite involved.
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_failure_if_not_initialized() {
   let reporter = yield getReporter("failure_if_not_initialized");
-  yield reporter._shutdown();
+  reporter._shutdown();
 
   let error = false;
   try {
     yield reporter.requestDataUpload();
   } catch (ex) {
     error = true;
     do_check_true(ex.message.contains("Not initialized."));
   } finally {
@@ -904,25 +905,23 @@ add_task(function test_failure_if_not_in
     error = false;
   }
 
   // getJSONPayload always works (to facilitate error upload).
   yield reporter.getJSONPayload();
 });
 
 add_task(function test_upload_on_init_failure() {
-  let MESSAGE = "Fake error during provider manager initialization." + Math.random();
-
   let server = new BagheeraServer();
   server.start();
   let reporter = yield getHealthReporter("upload_on_init_failure", server.serverURI, true);
   server.createNamespace(reporter.serverNamespace);
 
   reporter.onInitializeProviderManagerFinished = function () {
-    throw new Error(MESSAGE);
+    throw new Error("Fake error during provider manager initialization.");
   };
 
   let deferred = Promise.defer();
 
   let oldOnResult = reporter._onBagheeraResult;
   Object.defineProperty(reporter, "_onBagheeraResult", {
     value: function (request, isDelete, date, result) {
       do_check_false(isDelete);
@@ -950,19 +949,19 @@ add_task(function test_upload_on_init_fa
   yield deferred.promise;
 
   do_check_true(server.hasDocument(reporter.serverNamespace, reporter.lastSubmitID));
   let doc = server.getDocument(reporter.serverNamespace, reporter.lastSubmitID);
   do_check_true("notInitialized" in doc);
   do_check_eq(doc.notInitialized, 1);
   do_check_true("errors" in doc);
   do_check_eq(doc.errors.length, 1);
-  do_check_true(doc.errors[0].contains(MESSAGE));
+  do_check_true(doc.errors[0].contains("Fake error during provider manager initialization"));
 
-  yield reporter._shutdown();
+  reporter._shutdown();
   yield shutdownServer(server);
 });
 
 add_task(function test_state_prefs_conversion_simple() {
   let reporter = getHealthReporter("state_prefs_conversion");
   let prefs = reporter._prefs;
 
   let lastSubmit = new Date();
@@ -978,17 +977,17 @@ add_task(function test_state_prefs_conve
     do_check_eq(reporter._state.lastPingDate.getTime(), reporter.lastPingDate.getTime());
     do_check_eq(reporter._state.lastSubmitID, reporter.lastSubmitID);
     do_check_true(reporter.haveRemoteData());
 
     // User set preferences should have been wiped out.
     do_check_false(prefs.isSet("lastSubmitID"));
     do_check_false(prefs.isSet("lastPingTime"));
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // If the saved JSON file does not contain an object, we should reset
 // automatically.
 add_task(function test_state_no_json_object() {
   let reporter = getHealthReporter("state_shared");
   yield CommonUtils.writeJSON("hello", reporter._state._filename);
@@ -1000,17 +999,17 @@ add_task(function test_state_no_json_obj
     do_check_null(reporter.lastSubmitID);
 
     let o = yield CommonUtils.readJSON(reporter._state._filename);
     do_check_eq(typeof(o), "object");
     do_check_eq(o.v, 1);
     do_check_eq(o.lastPingTime, 0);
     do_check_eq(o.remoteIDs.length, 0);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // If we encounter a future version, we reset state to the current version.
 add_task(function test_state_future_version() {
   let reporter = getHealthReporter("state_shared");
   yield CommonUtils.writeJSON({v: 2, remoteIDs: ["foo"], lastPingTime: 2412},
                               reporter._state._filename);
@@ -1021,17 +1020,17 @@ add_task(function test_state_future_vers
     do_check_null(reporter.lastSubmitID);
 
     // While the object is updated, we don't save the file.
     let o = yield CommonUtils.readJSON(reporter._state._filename);
     do_check_eq(o.v, 2);
     do_check_eq(o.lastPingTime, 2412);
     do_check_eq(o.remoteIDs.length, 1);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // Test recovery if the state file contains invalid JSON.
 add_task(function test_state_invalid_json() {
   let reporter = getHealthReporter("state_shared");
 
   let encoder = new TextEncoder();
@@ -1040,17 +1039,17 @@ add_task(function test_state_invalid_jso
   yield OS.File.writeAtomic(path, arr, {tmpPath: path + ".tmp"});
 
   try {
     yield reporter.init();
 
     do_check_eq(reporter.lastPingDate.getTime(), 0);
     do_check_null(reporter.lastSubmitID);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 add_task(function test_state_multiple_remote_ids() {
   let [reporter, server] = yield getReporterAndServer("state_multiple_remote_ids");
   let documents = [
     [reporter.serverNamespace, "one", "{v:1}"],
     [reporter.serverNamespace, "two", "{v:2}"],
@@ -1081,17 +1080,17 @@ add_task(function test_state_multiple_re
     do_check_true(reporter.lastPingDate.getTime() > now.getTime());
 
     let o = yield CommonUtils.readJSON(reporter._state._filename);
     do_check_eq(o.remoteIDs.length, 1);
     do_check_eq(o.remoteIDs[0], reporter._state.remoteIDs[0]);
     do_check_eq(o.lastPingTime, reporter.lastPingDate.getTime());
   } finally {
     yield shutdownServer(server);
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // If we have a state file then downgrade to prefs, the prefs should be
 // reimported and should supplement existing state.
 add_task(function test_state_downgrade_upgrade() {
   let reporter = getHealthReporter("state_shared");
 
@@ -1113,17 +1112,17 @@ add_task(function test_state_downgrade_u
     do_check_eq(reporter.lastPingDate.getTime(), now.getTime() + 1000);
     do_check_false(prefs.isSet("lastSubmitID"));
     do_check_false(prefs.isSet("lastPingTime"));
 
     let o = yield CommonUtils.readJSON(reporter._state._filename);
     do_check_eq(o.remoteIDs.length, 3);
     do_check_eq(o.lastPingTime, now.getTime() + 1000);
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
 
 // Missing client ID in state should be created on state load.
 add_task(function* test_state_create_client_id() {
   let reporter = getHealthReporter("state_create_client_id");
 
   yield CommonUtils.writeJSON({
--- a/services/healthreport/tests/xpcshell/test_provider_appinfo.js
+++ b/services/healthreport/tests/xpcshell/test_provider_appinfo.js
@@ -258,11 +258,11 @@ add_task(function test_healthreporter_in
 
     for (let [day, measurements] in Iterator(days)) {
       do_check_eq(Object.keys(measurements).length, 3);
       do_check_true("org.mozilla.appInfo.appinfo" in measurements);
       do_check_true("org.mozilla.appInfo.update" in measurements);
       do_check_true("org.mozilla.appInfo.versions" in measurements);
     }
   } finally {
-    yield reporter._shutdown();
+    reporter._shutdown();
   }
 });
--- a/services/metrics/storage.jsm
+++ b/services/metrics/storage.jsm
@@ -806,18 +806,17 @@ MetricsStorageSqliteBackend.prototype = 
   ],
 
   /**
    * Close the database connection.
    *
    * This should be called on all instances or the SQLite layer may complain
    * loudly. After this has been called, the connection cannot be used.
    *
-   * @return Promise<> A promise fulfilled once the connection is closed.
-   * This promise never rejects.
+   * @return Promise<>
    */
   close: function () {
     return Task.spawn(function doClose() {
       // There is some light magic involved here. First, we enqueue an
       // operation to ensure that all pending operations have the opportunity
       // to execute. We additionally execute a SQL operation. Due to the FIFO
       // execution order of issued statements, this will cause us to wait on
       // any outstanding statements before closing.
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4394,16 +4394,24 @@
   "HEALTHREPORT_INIT_MS": {
     "expires_in_version": "never",
     "kind": "exponential",
     "high": "20000",
     "n_buckets": 15,
     "extended_statistics_ok": true,
     "description": "Time (ms) spent to initialize Firefox Health Report service."
   },
+  "HEALTHREPORT_SHUTDOWN_DELAY_MS": {
+    "expires_in_version": "never",
+    "kind": "exponential",
+    "high": "20000",
+    "n_buckets": 15,
+    "extended_statistics_ok": true,
+    "description": "Time (ms) that Firefox Health Report delays application shutdown by."
+  },
   "HEALTHREPORT_GENERATE_JSON_PAYLOAD_MS": {
     "expires_in_version": "never",
     "kind": "exponential",
     "high": "30000",
     "n_buckets": 20,
     "extended_statistics_ok": true,
     "description": "Time (ms) it takes to obtain and format a Health Report JSON payload."
   },