From 11e72d29e52f4d7d83f774f18295c5ac306230e2 Mon Sep 17 00:00:00 2001 From: Yura Zenevich Date: Tue, 30 Apr 2013 14:07:40 +0200 Subject: [PATCH] [PATCH 1/2] Bug 833286 - added improvements to atomic backup of sessionstore.js. r=yoric, ttaubert --- .../components/sessionstore/src/SessionStore.jsm | 45 +++++-- .../components/sessionstore/src/_SessionFile.jsm | 131 ++++++++++++++----- 2 files changed, 127 insertions(+), 49 deletions(-) --- .../sessionstore/src/SessionStore.jsm | 45 ++++-- .../sessionstore/src/_SessionFile.jsm | 131 +++++++++++++----- 2 files changed, 127 insertions(+), 49 deletions(-) diff --git a/browser/components/sessionstore/src/SessionStore.jsm b/browser/components/sessionstore/src/SessionStore.jsm index 27180fdbea67..08e0f7a74e76 100644 --- a/browser/components/sessionstore/src/SessionStore.jsm +++ b/browser/components/sessionstore/src/SessionStore.jsm @@ -459,13 +459,10 @@ let SessionStoreInternal = { catch (ex) { debug("The session file is invalid: " + ex); } } - if (this._resume_from_crash) { - // Launch background copy of the session file. Note that we do - // not have race conditions here as _SessionFile guarantees - // that any I/O operation is completed before proceeding to - // the next I/O operation. - _SessionFile.createBackupCopy(); - } + // A Lazy getter for the sessionstore.js backup promise. + XPCOMUtils.defineLazyGetter(this, "_backupSessionFileOnce", function () { + return _SessionFile.createBackupCopy(); + }); // at this point, we've as good as resumed the session, so we can // clear the resume_session_once flag, if it's set @@ -3758,13 +3755,33 @@ let SessionStoreInternal = { return; } - let self = this; - _SessionFile.write(data).then( - function onSuccess() { - self._lastSaveTime = Date.now(); - Services.obs.notifyObservers(null, "sessionstore-state-write-complete", ""); - } - ); + let promise; + // If "sessionstore.resume_from_crash" is true, attempt to backup the + // session file first, before writing to it. + if (this._resume_from_crash) { + // Note that we do not have race conditions here as _SessionFile + // guarantees that any I/O operation is completed before proceeding to + // the next I/O operation. + // Note backup happens only once, on initial save. + promise = this._backupSessionFileOnce; + } else { + promise = Promise.resolve(); + } + + // Attempt to write to the session file (potentially, depending on + // "sessionstore.resume_from_crash" preference, after successful backup). + promise = promise.then(function onSuccess() { + // Write (atomically) to a session file, using a tmp file. + return _SessionFile.write(data); + }); + + // Once the session file is successfully updated, save the time stamp of the + // last save and notify the observers. + promise = promise.then(() => { + this._lastSaveTime = Date.now(); + Services.obs.notifyObservers(null, "sessionstore-state-write-complete", + ""); + }); }, /* ........ Auxiliary Functions .............. */ diff --git a/browser/components/sessionstore/src/_SessionFile.jsm b/browser/components/sessionstore/src/_SessionFile.jsm index 88c5fc6a1d9a..b723c1d48f06 100644 --- a/browser/components/sessionstore/src/_SessionFile.jsm +++ b/browser/components/sessionstore/src/_SessionFile.jsm @@ -49,6 +49,10 @@ XPCOMUtils.defineLazyServiceGetter(this, "Telemetry", XPCOMUtils.defineLazyGetter(this, "gEncoder", function () { return new TextEncoder(); }); +// A decoder. +XPCOMUtils.defineLazyGetter(this, "gDecoder", function () { + return new TextDecoder(); +}); this._SessionFile = { /** @@ -144,61 +148,118 @@ let SessionFileInternal = { */ backupPath: OS.Path.join(OS.Constants.Path.profileDir, "sessionstore.bak"), + /** + * Utility function to safely read a file synchronously. + * @param aPath + * A path to read the file from. + * @returns string if successful, undefined otherwise. + */ + readAuxSync: function ssfi_readAuxSync(aPath) { + let text; + try { + let file = new FileUtils.File(aPath); + let chan = NetUtil.newChannel(file); + let stream = chan.open(); + text = NetUtil.readInputStreamToString(stream, stream.available(), + {charset: "utf-8"}); + } catch (e if e.result == Components.results.NS_ERROR_FILE_NOT_FOUND) { + // Ignore exceptions about non-existent files. + } catch (ex) { + // Any other error. + Cu.reportError(ex); + } finally { + return text; + } + }, + /** * Read the sessionstore file synchronously. * * This function is meant to serve as a fallback in case of race * between a synchronous usage of the API and asynchronous * initialization. + * + * In case if sessionstore.js file does not exist or is corrupted (something + * happened between backup and write), attempt to read the sessionstore.bak + * instead. */ syncRead: function ssfi_syncRead() { - let text; - let exn; + // Start measuring the duration of the synchronous read. TelemetryStopwatch.start("FX_SESSION_RESTORE_SYNC_READ_FILE_MS"); - try { - let file = new FileUtils.File(this.path); - let chan = NetUtil.newChannel(file); - let stream = chan.open(); - text = NetUtil.readInputStreamToString(stream, stream.available(), {charset: "utf-8"}); - } catch (e if e.result == Components.results.NS_ERROR_FILE_NOT_FOUND) { - return ""; - } catch(ex) { - exn = ex; - } finally { - TelemetryStopwatch.finish("FX_SESSION_RESTORE_SYNC_READ_FILE_MS"); + // First read the sessionstore.js. + let text = this.readAuxSync(this.path); + if (typeof text === "undefined") { + // If sessionstore.js does not exist or is corrupted, read sessionstore.bak. + text = this.readAuxSync(this.backupPath); } - if (exn) { - Cu.reportError(exn); - return ""; - } - return text; + // Finish the telemetry probe and return an empty string. + TelemetryStopwatch.finish("FX_SESSION_RESTORE_SYNC_READ_FILE_MS"); + return text || ""; }, - read: function ssfi_read() { - let refObj = {}; + /** + * Utility function to safely read a file asynchronously. + * @param aPath + * A path to read the file from. + * @param aReadOptions + * Read operation options. + * |outExecutionDuration| option will be reused and can be + * incrementally updated by the worker process. + * @returns string if successful, undefined otherwise. + */ + readAux: function ssfi_readAux(aPath, aReadOptions) { let self = this; - return TaskUtils.spawn(function task() { - TelemetryStopwatch.start("FX_SESSION_RESTORE_READ_FILE_MS", refObj); + return TaskUtils.spawn(function () { let text; try { - let bytes = yield OS.File.read(self.path); - text = new TextDecoder().decode(bytes); - TelemetryStopwatch.finish("FX_SESSION_RESTORE_READ_FILE_MS", refObj); + let bytes = yield OS.File.read(aPath, undefined, aReadOptions); + text = gDecoder.decode(bytes); + // If the file is read successfully, add a telemetry probe based on + // the updated duration value of the |outExecutionDuration| option. + let histogram = Telemetry.getHistogramById( + "FX_SESSION_RESTORE_READ_FILE_MS"); + histogram.add(aReadOptions.outExecutionDuration); + } catch (ex if self._isNoSuchFile(ex)) { + // Ignore exceptions about non-existent files. } catch (ex) { - if (self._isNoSuchFile(ex)) { - // The file does not exist, this is perfectly valid - TelemetryStopwatch.finish("FX_SESSION_RESTORE_READ_FILE_MS", refObj); - } else { - // Any other error: let's not measure with telemetry - TelemetryStopwatch.cancel("FX_SESSION_RESTORE_READ_FILE_MS", refObj); - Cu.reportError(ex); - } - text = ""; + Cu.reportError(ex); } throw new Task.Result(text); }); }, + /** + * Read the sessionstore file asynchronously. + * + * In case sessionstore.js file does not exist or is corrupted (something + * happened between backup and write), attempt to read the sessionstore.bak + * instead. + */ + read: function ssfi_read() { + let self = this; + return TaskUtils.spawn(function task() { + // Specify |outExecutionDuration| option to hold the combined duration of + // the asynchronous reads off the main thread (of both sessionstore.js and + // sessionstore.bak, if necessary). If sessionstore.js does not exist or + // is corrupted, |outExecutionDuration| will register the time it took to + // attempt to read the file. It will then be subsequently incremented by + // the read time of sessionsore.bak. + let readOptions = { + outExecutionDuration: null + }; + // First read the sessionstore.js. + let text = yield self.readAux(self.path, readOptions); + if (typeof text === "undefined") { + // If sessionstore.js does not exist or is corrupted, read the + // sessionstore.bak. + text = yield self.readAux(self.backupPath, readOptions); + } + // Return either the content of the sessionstore.bak if it was read + // successfully or an empty string otherwise. + throw new Task.Result(text || ""); + }); + }, + write: function ssfi_write(aData) { let refObj = {}; let self = this; @@ -232,7 +293,7 @@ let SessionFileInternal = { let self = this; return TaskUtils.spawn(function task() { try { - yield OS.File.copy(self.path, self.backupPath, backupCopyOptions); + yield OS.File.move(self.path, self.backupPath, backupCopyOptions); Telemetry.getHistogramById("FX_SESSION_RESTORE_BACKUP_FILE_MS").add( backupCopyOptions.outExecutionDuration); } catch (ex if self._isNoSuchFile(ex)) {