Bug 1332024 - Add a method to explicitly finalize `JSONFile` objects. r=Paolo
MozReview-Commit-ID: QaRqLBGEft
--- a/toolkit/modules/JSONFile.jsm
+++ b/toolkit/modules/JSONFile.jsm
@@ -81,34 +81,40 @@ const kSaveDelayMs = 1500;
* - saveDelayMs: Number indicating the delay (in milliseconds) between a
* change to the data and the related save operation. The
* default value will be applied if omitted.
* - beforeSave: Promise-returning function triggered just before the
* data is written to disk. This can be used to create any
* intermediate directories before saving. The file will
* not be saved if the promise rejects or the function
* throws an exception.
+ * - finalizeAt: An `AsyncShutdown` phase or barrier client that should
+ * automatically finalize the file when triggered. Defaults
+ * to `profileBeforeChange`; exposed as an option for
+ * testing.
*/
function JSONFile(config) {
this.path = config.path;
if (typeof config.dataPostProcessor === "function") {
this._dataPostProcessor = config.dataPostProcessor;
}
if (typeof config.beforeSave === "function") {
this._beforeSave = config.beforeSave;
}
if (config.saveDelayMs === undefined) {
config.saveDelayMs = kSaveDelayMs;
}
this._saver = new DeferredTask(() => this._save(), config.saveDelayMs);
- AsyncShutdown.profileBeforeChange.addBlocker("JSON store: writing data",
- () => this._saver.finalize());
+ this._finalizeAt = config.finalizeAt || AsyncShutdown.profileBeforeChange;
+ this._finalizeInternalBound = this._finalizeInternal.bind(this);
+ this._finalizeAt.addBlocker("JSON store: writing data",
+ this._finalizeInternalBound);
}
JSONFile.prototype = {
/**
* String containing the file path where data should be saved.
*/
path: "",
@@ -123,16 +129,23 @@ JSONFile.prototype = {
_saver: null,
/**
* Internal data object.
*/
_data: null,
/**
+ * Internal fields used during finalization.
+ */
+ _finalizeAt: null,
+ _finalizePromise: null,
+ _finalizeInternalBound: null,
+
+ /**
* Serializable object containing the data. This is populated directly with
* the data loaded from the file, and is saved without modifications.
*
* The raw data should be manipulated synchronously, without waiting for the
* event loop or for promise resolution, so that the saved file is always
* consistent.
*/
get data() {
@@ -275,11 +288,50 @@ JSONFile.prototype = {
yield OS.File.writeAtomic(this.path, bytes,
{ tmpPath: this.path + ".tmp" });
}),
/**
* Synchronously work on the data just loaded into memory.
*/
_processLoadedData(data) {
+ if (this._finalizePromise) {
+ // It's possible for `load` to race with `finalize`. In that case, don't
+ // process or set the loaded data.
+ return;
+ }
this.data = this._dataPostProcessor ? this._dataPostProcessor(data) : data;
},
+
+ /**
+ * Finishes persisting data to disk and resets all state for this file.
+ *
+ * @return {Promise}
+ * @resolves When the object is finalized.
+ */
+ _finalizeInternal() {
+ if (this._finalizePromise) {
+ // Finalization already in progress; return the pending promise. This is
+ // possible if `finalize` is called concurrently with shutdown.
+ return this._finalizePromise;
+ }
+ this._finalizePromise = Task.spawn(function* () {
+ yield this._saver.finalize();
+ this._data = null;
+ this.dataReady = false;
+ }.bind(this));
+ return this._finalizePromise;
+ },
+
+ /**
+ * Ensures that all data is persisted to disk, and prevents future calls to
+ * `saveSoon`. This is called automatically on shutdown, but can also be
+ * called explicitly when the file is no longer needed.
+ */
+ finalize: Task.async(function* () {
+ if (this._finalizePromise) {
+ throw new Error(`The file ${this.path} has already been finalized`);
+ }
+ // Wait for finalization before removing the shutdown blocker.
+ yield this._finalizeInternal();
+ this._finalizeAt.removeBlocker(this._finalizeInternalBound);
+ }),
};
--- a/toolkit/modules/tests/xpcshell/test_JSONFile.js
+++ b/toolkit/modules/tests/xpcshell/test_JSONFile.js
@@ -5,16 +5,18 @@
"use strict";
// Globals
const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
+ "resource://gre/modules/AsyncShutdown.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "DownloadPaths",
"resource://gre/modules/DownloadPaths.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
"resource://gre/modules/FileUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "OS",
"resource://gre/modules/osfile.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
"resource://gre/modules/JSONFile.jsm");
@@ -296,8 +298,63 @@ add_task(function* test_beforeSave_rejec
};
storeForSave.saveSoon();
});
yield Assert.rejects(promiseSave, function(ex) {
return ex.message == "oops";
});
});
+
+add_task(function* test_finalize() {
+ let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+ let barrier = new AsyncShutdown.Barrier("test-auto-finalize");
+ let storeForSave = new JSONFile({
+ path,
+ saveDelayMs: 2000,
+ finalizeAt: barrier.client,
+ });
+ yield storeForSave.load();
+ storeForSave.data = TEST_DATA;
+ storeForSave.saveSoon();
+
+ let promiseFinalize = storeForSave.finalize();
+ yield Assert.rejects(storeForSave.finalize(), /has already been finalized$/);
+ yield promiseFinalize;
+ do_check_false(storeForSave.dataReady);
+
+ // Finalization removes the blocker, so waiting should not log an unhandled
+ // error even though the object has been explicitly finalized.
+ yield barrier.wait();
+
+ let storeForLoad = new JSONFile({ path });
+ yield storeForLoad.load();
+ do_check_matches(storeForLoad.data, TEST_DATA);
+});
+
+add_task(function* test_finalize_on_shutdown() {
+ let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+ let barrier = new AsyncShutdown.Barrier("test-finalize-shutdown");
+ let storeForSave = new JSONFile({
+ path,
+ saveDelayMs: 2000,
+ finalizeAt: barrier.client,
+ });
+ yield storeForSave.load();
+ storeForSave.data = TEST_DATA;
+ // Arm the saver, then simulate shutdown and ensure the file is
+ // automatically finalized.
+ storeForSave.saveSoon();
+
+ yield barrier.wait();
+ // It's possible for `finalize` to reject when called concurrently with
+ // shutdown. We don't distinguish between explicit `finalize` calls and
+ // finalization on shutdown because we expect most consumers to rely on the
+ // latter. However, this behavior can be safely changed if needed.
+ yield Assert.rejects(storeForSave.finalize(), /has already been finalized$/);
+ do_check_false(storeForSave.dataReady);
+
+ let storeForLoad = new JSONFile({ path });
+ yield storeForLoad.load();
+ do_check_matches(storeForLoad.data, TEST_DATA);
+});