Bug 1402267 - Restart the SessionWorker each time there are failures reported as much as defined in the 'browser.sessionstore.max_write_failures' pref. r?ttaubert draft
authorMike de Boer <mdeboer@mozilla.com>
Tue, 17 Oct 2017 11:59:33 +0200
changeset 681453 6f23983c4b3763f756aa3d25ff2d978bb5dad566
parent 681452 0cbf7659af5a3838da4abb6bfbcc58ad36c7e801
child 681454 f5a9abe6f46b7fd0eaf26e5d260145fbc7059938
child 681455 aded9609532d33d65e7c4d8683d135139f7b6346
push id84839
push usermdeboer@mozilla.com
push dateTue, 17 Oct 2017 10:05:48 +0000
reviewersttaubert
bugs1402267
milestone58.0a1
Bug 1402267 - Restart the SessionWorker each time there are failures reported as much as defined in the 'browser.sessionstore.max_write_failures' pref. r?ttaubert MozReview-Commit-ID: 91vOcbmhFmj
browser/app/profile/firefox.js
browser/components/sessionstore/SessionFile.jsm
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -885,16 +885,18 @@ pref("browser.sessionstore.debug", false
 // Causes SessionStore to ignore non-final update messages from
 // browser tabs that were not caused by a flush from the parent.
 // This is a testing flag and should not be used by end-users.
 pref("browser.sessionstore.debug.no_auto_updates", false);
 // Forget closed windows/tabs after two weeks
 pref("browser.sessionstore.cleanup.forget_closed_after", 1209600000);
 // Maximum number of bytes of DOMSessionStorage data we collect per origin.
 pref("browser.sessionstore.dom_storage_limit", 2048);
+// Amount of failed SessionFile writes until we restart the worker.
+pref("browser.sessionstore.max_write_failures", 5);
 
 // allow META refresh by default
 pref("accessibility.blockautorefresh", false);
 
 // Whether useAsyncTransactions is enabled or not.
 // Currently we only enable them for nightly.
 #ifdef NIGHTLY_BUILD
 pref("browser.places.useAsyncTransactions", true);
--- a/browser/components/sessionstore/SessionFile.jsm
+++ b/browser/components/sessionstore/SessionFile.jsm
@@ -53,16 +53,19 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   "resource:///modules/sessionstore/SessionStore.jsm");
 
 const PREF_UPGRADE_BACKUP = "browser.sessionstore.upgradeBackup.latestBuildID";
 const PREF_MAX_UPGRADE_BACKUPS = "browser.sessionstore.upgradeBackup.maxUpgradeBackups";
 
 const PREF_MAX_SERIALIZE_BACK = "browser.sessionstore.max_serialize_back";
 const PREF_MAX_SERIALIZE_FWD = "browser.sessionstore.max_serialize_forward";
 
+XPCOMUtils.defineLazyPreferenceGetter(this, "kMaxWriteFailures",
+  "browser.sessionstore.max_write_failures", 5);
+
 this.SessionFile = {
   /**
    * Read the contents of the session file, asynchronously.
    */
   read() {
     return SessionFileInternal.read();
   },
   /**
@@ -182,16 +185,22 @@ var SessionFileInternal = {
   // Number of successful calls to `write`.
   // Used for error reporting.
   _successes: 0,
 
   // Number of failed calls to `write`.
   // Used for error reporting.
   _failures: 0,
 
+  // Object that keeps statistics that should help us make informed decisions
+  // about the current status of the worker.
+  _workerHealth: {
+    failures: 0
+  },
+
   // Resolved once initialization is complete.
   // The promise never rejects.
   _deferredInitialized: PromiseUtils.defer(),
 
   // `true` once we have started initialization, i.e. once something
   // has been scheduled that will eventually resolve `_deferredInitialized`.
   _initializationStarted: false,
 
@@ -226,17 +235,18 @@ var SessionFileInternal = {
           path = this.Paths[key];
           options.compression = "lz4";
         }
         let source = await OS.File.read(path, options);
         let parsed = JSON.parse(source);
 
         if (!SessionStore.isFormatVersionCompatible(parsed.version || ["sessionrestore", 0] /* fallback for old versions*/)) {
           // Skip sessionstore files that we don't understand.
-          Cu.reportError("Cannot extract data from Session Restore file " + path + ". Wrong format/version: " + JSON.stringify(parsed.version) + ".");
+          Cu.reportError("Cannot extract data from Session Restore file " + path +
+            ". Wrong format/version: " + JSON.stringify(parsed.version) + ".");
           continue;
         }
         result = {
           origin: key,
           source,
           parsed,
           useOldExtension
         };
@@ -327,16 +337,29 @@ var SessionFileInternal = {
       // Initialization will be complete once `this._deferredInitialized.promise`
       // resolves.
       this.read();
     }
     await this._deferredInitialized.promise;
     return SessionWorker.post(...args);
   },
 
+  /**
+   * For good measure, terminate the worker when we've had over `kMaxWriteFailures`
+   * amount of failures to deal with. This will spawn a fresh worker upon the next
+   * write.
+   * This also resets the `_workerHealth` stats.
+   */
+  _checkWorkerHealth() {
+    if (this._workerHealth.failures >= kMaxWriteFailures) {
+      SessionWorker.terminate();
+      this._workerHealth.failures = 0;
+    }
+  },
+
   write(aData) {
     if (RunState.isClosed) {
       return Promise.reject(new Error("SessionFile is closed"));
     }
 
     let isFinalWrite = false;
     if (RunState.isClosing) {
       // If shutdown has started, we will want to stop receiving
@@ -362,16 +385,17 @@ var SessionFileInternal = {
         // in preferences.
         Services.prefs.setCharPref(PREF_UPGRADE_BACKUP,
           Services.appinfo.platformBuildID);
       }
     }, err => {
       // Catch and report any errors.
       console.error("Could not write session state file ", err, err.stack);
       this._failures++;
+      this._workerHealth.failures++;
       // By not doing anything special here we ensure that |promise| cannot
       // be rejected anymore. The shutdown/cleanup code at the end of the
       // function will thus always be executed.
     });
 
     // Ensure that we can write sessionstore.js cleanly before the profile
     // becomes unaccessible.
     AsyncShutdown.profileBeforeChange.addBlocker(
@@ -390,16 +414,18 @@ var SessionFileInternal = {
     // We ensured that by having a reject handler that reports the failure but
     // doesn't forward the rejection.
     return promise.then(() => {
       // Remove the blocker, no matter if writing failed or not.
       AsyncShutdown.profileBeforeChange.removeBlocker(promise);
 
       if (isFinalWrite) {
         Services.obs.notifyObservers(null, "sessionstore-final-state-write-complete");
+      } else {
+        this._checkWorkerHealth();
       }
     });
   },
 
   wipe() {
     return this._postToWorker("wipe");
   },