Bug 1190627 - Part 1 - Use temp file for synchronous writes, too. r=margaret draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 17 Apr 2016 18:22:36 +0200
changeset 377041 699999a55d8697ac4f978209a21f24bcce6d1b79
parent 377040 2dac3785f1406a65b4c239bcb2940dcb8aeca490
child 377042 74bfa2a4752cd2f2ca0e96159502d372522d008d
push id20736
push usermozilla@buttercookie.de
push dateThu, 09 Jun 2016 14:50:37 +0000
reviewersmargaret
bugs1190627
milestone50.0a1
Bug 1190627 - Part 1 - Use temp file for synchronous writes, too. r=margaret Currently, sync writes go directly to the destination file, so an interrupted write will leave the session store data in an inconsistent state. To minimise the incidence of this occurring as far as possible, we now mimic the behaviour of atomicWrite when a tmpPath is set and write to a temporary file which is then renamed to the actual destination file after writing has finished. MozReview-Commit-ID: 3f3z1s0hfl8
mobile/android/components/SessionStore.js
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -72,18 +72,20 @@ SessionStore.prototype = {
   _notifyClosedTabs: false,
 
   init: function ss_init() {
     loggingEnabled = Services.prefs.getBoolPref("browser.sessionstore.debug_logging");
 
     // Get file references
     this._sessionFile = Services.dirsvc.get("ProfD", Ci.nsILocalFile);
     this._sessionFileBackup = this._sessionFile.clone();
+    this._sessionFileTemp = this._sessionFile.clone();
     this._sessionFile.append("sessionstore.js");
     this._sessionFileBackup.append("sessionstore.bak");
+    this._sessionFileTemp.append(this._sessionFile.leafName + ".tmp");
 
     this._loadState = STATE_STOPPED;
 
     this._interval = Services.prefs.getIntPref("browser.sessionstore.interval");
     this._maxTabsUndo = Services.prefs.getIntPref("browser.sessionstore.max_tabs_undo");
 
     // Copy changes in Gecko settings to their Java counterparts,
     // so the startup code can access them
@@ -95,16 +97,17 @@ SessionStore.prototype = {
       SharedPreferences.forApp().setIntPref(PREFS_MAX_CRASH_RESUMES,
         Services.prefs.getIntPref(PREFS_MAX_CRASH_RESUMES));
     }, false);
   },
 
   _clearDisk: function ss_clearDisk() {
     OS.File.remove(this._sessionFile.path);
     OS.File.remove(this._sessionFileBackup.path);
+    OS.File.remove(this._sessionFileTemp.path);
   },
 
   observe: function ss_observe(aSubject, aTopic, aData) {
     let self = this;
     let observerService = Services.obs;
     switch (aTopic) {
       case "app-startup":
         observerService.addObserver(this, "final-ui-startup", true);
@@ -818,17 +821,17 @@ SessionStore.prototype = {
 
     // Write only non-private data to disk
     if (normalData.windows[0] && normalData.windows[0].tabs) {
       log("_saveState() writing normal data, " +
            normalData.windows[0].tabs.length + " tabs in window[0]");
     } else {
       log("_saveState() writing empty normal data");
     }
-    this._writeFile(this._sessionFile, normalData, aAsync);
+    this._writeFile(this._sessionFile, this._sessionFileTemp, normalData, aAsync);
 
     // If we have private data, send it to Java; otherwise, send null to
     // indicate that there is no private data
     Messaging.sendRequest({
       type: "PrivateBrowsing:Data",
       session: (privateData.windows.length > 0 && privateData.windows[0].tabs.length > 0) ? JSON.stringify(privateData) : null
     });
 
@@ -902,34 +905,35 @@ SessionStore.prototype = {
       }
     }
   },
 
   /**
    * Writes the session state to a disk file, while doing some telemetry and notification
    * bookkeeping.
    * @param aFile nsIFile used for saving the session
+   * @param aFileTemp nsIFile used as a temporary file in writing the data
    * @param aData JSON session state
    * @param aAsync boolelan used to determine the method of saving the state
    */
-  _writeFile: function ss_writeFile(aFile, aData, aAsync) {
+  _writeFile: function ss_writeFile(aFile, aFileTemp, aData, aAsync) {
     TelemetryStopwatch.start("FX_SESSION_RESTORE_SERIALIZE_DATA_MS");
     let state = JSON.stringify(aData);
     TelemetryStopwatch.finish("FX_SESSION_RESTORE_SERIALIZE_DATA_MS");
 
     // Convert data string to a utf-8 encoded array buffer
     let buffer = new TextEncoder().encode(state);
     Services.telemetry.getHistogramById("FX_SESSION_RESTORE_FILE_SIZE_BYTES").add(buffer.byteLength);
 
     Services.obs.notifyObservers(null, "sessionstore-state-write", "");
     let startWriteMs = Cu.now();
 
     log("_writeFile(aAsync = " + aAsync + "), _pendingWrite = " + this._pendingWrite);
     let pendingWrite = this._pendingWrite;
-    this._write(aFile, buffer, aAsync).then(() => {
+    this._write(aFile, aFileTemp, buffer, aAsync).then(() => {
       let stopWriteMs = Cu.now();
 
       // Make sure this._pendingWrite is the same value it was before we
       // fired off the async write. If the count is different, another write
       // is pending, so we shouldn't reset this._pendingWrite yet.
       if (pendingWrite === this._pendingWrite) {
         this._pendingWrite = 0;
       }
@@ -941,33 +945,37 @@ SessionStore.prototype = {
       Services.telemetry.getHistogramById("FX_SESSION_RESTORE_WRITE_FILE_MS").add(Math.round(stopWriteMs - startWriteMs));
       Services.obs.notifyObservers(null, "sessionstore-state-write-complete", "");
     });
   },
 
   /**
    * Writes the session state to a disk file, using async or sync methods
    * @param aFile nsIFile used for saving the session
+   * @param aFileTemp nsIFile used as a temporary file in writing the data
    * @param aBuffer UTF-8 encoded ArrayBuffer of the session state
    * @param aAsync boolelan used to determine the method of saving the state
    * @return Promise that resolves when the file has been written
    */
-  _write: function ss_write(aFile, aBuffer, aAsync) {
+  _write: function ss_write(aFile, aFileTemp, aBuffer, aAsync) {
     // Use async file writer and just return it's promise
     if (aAsync) {
       log("_write() writing asynchronously");
-      return OS.File.writeAtomic(aFile.path, aBuffer, { tmpPath: aFile.path + ".tmp" });
+      return OS.File.writeAtomic(aFile.path, aBuffer, { tmpPath: aFileTemp.path });
     }
 
     // Convert buffer to an encoded string and sync write to disk
     let bytes = String.fromCharCode.apply(null, new Uint16Array(aBuffer));
     let stream = Cc["@mozilla.org/network/file-output-stream;1"].createInstance(Ci.nsIFileOutputStream);
-    stream.init(aFile, 0x02 | 0x08 | 0x20, 0o666, 0);
+    stream.init(aFileTemp, 0x02 | 0x08 | 0x20, 0o666, 0);
     stream.write(bytes, bytes.length);
     stream.close();
+    // Mimic writeAtomic behaviour when tmpPath is set and write
+    // to a temp file which is then renamed at the end.
+    aFileTemp.renameTo(null, aFile.leafName);
     log("_write() writing synchronously");
 
     // Return a resolved promise to make the caller happy
     return Promise.resolve();
   },
 
   _updateCrashReportURL: function ss_updateCrashReportURL(aWindow) {
     let crashReporterBuilt = "nsICrashReporter" in Ci && Services.appinfo instanceof Ci.nsICrashReporter;