Bug 828149 - Cancel timer during application shutdown. r=rnewman, a=lsblakk
authorGregory Szorc <gps@mozilla.com>
Mon, 14 Jan 2013 18:01:53 -0800
changeset 127245 71009680316b5518a80724d49650d072c9ddb1c0
parent 127244 0836a971d9265a627750af069f66c969603ed56e
child 127246 a2bc1943f3e08c1623023b273055caf05aac4c26
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, lsblakk
bugs828149
milestone20.0a2
Bug 828149 - Cancel timer during application shutdown. r=rnewman, a=lsblakk
services/datareporting/DataReportingService.js
services/datareporting/modules-testing/mocks.jsm
services/datareporting/policy.jsm
services/datareporting/tests/xpcshell/test_policy.js
--- a/services/datareporting/DataReportingService.js
+++ b/services/datareporting/DataReportingService.js
@@ -48,16 +48,18 @@ const DEFAULT_LOAD_DELAY_MSEC = 10 * 100
  *
  * Shutdown of the `HealthReporter` instance is handled completely within the
  * instance (it registers observers on initialization). See the notes on that
  * type for more.
  */
 this.DataReportingService = function () {
   this.wrappedJSObject = this;
 
+  this._quitting = false;
+
   this._os = Cc["@mozilla.org/observer-service;1"]
                .getService(Ci.nsIObserverService);
 }
 
 DataReportingService.prototype = Object.freeze({
   classID: Components.ID("{41f6ae36-a79f-4613-9ac3-915e70f83789}"),
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
@@ -125,27 +127,47 @@ DataReportingService.prototype = Object.
         let delayInterval = this._prefs.get("service.loadDelayMsec") ||
                             DEFAULT_LOAD_DELAY_MSEC;
 
         // Delay service loading a little more so things have an opportunity
         // to cool down first.
         this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
         this.timer.initWithCallback({
           notify: function notify() {
+            delete this.timer;
+
+            // There could be a race between "quit-application" firing and
+            // this callback being invoked. We close that door.
+            if (this._quitting) {
+              return;
+            }
+
             // Side effect: instantiates the reporter instance if not already
             // accessed.
+            //
+            // The instance installs its own shutdown observers. So, we just
+            // fire and forget: it will clean itself up.
             let reporter = this.healthReporter;
-            delete this.timer;
           }.bind(this),
         }, delayInterval, this.timer.TYPE_ONE_SHOT);
 
         break;
 
       case "quit-application":
         this._os.removeObserver(this, "quit-application");
+        this._quitting = true;
+
+        // Shutdown doesn't clear pending timers. So, we need to explicitly
+        // cancel our health reporter initialization timer or else it will
+        // attempt initialization after shutdown has commenced. This would
+        // likely lead to stalls or crashes.
+        if (this.timer) {
+          this.timer.cancel();
+        }
+
         this.policy.stopPolling();
         break;
     }
   },
 
   /**
    * The HealthReporter instance associated with this service.
    *