From 8f5cecbacd2ebbb25500d2dc4c8ffbe5f55dd1bf Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 26 Jan 2018 11:15:35 -0800 Subject: [PATCH] Use a queue to prevent concurrent async writeFile calls --- spec/config-spec.js | 4 +--- src/application-delegate.js | 17 ++++++++++++++--- src/config-file.js | 37 ++++++++++++++++++++++++++----------- src/config.js | 2 +- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/spec/config-spec.js b/spec/config-spec.js index 0baa3c229..3754e517f 100644 --- a/spec/config-spec.js +++ b/spec/config-spec.js @@ -177,13 +177,11 @@ describe('Config', () => { it("saves the user's config to disk after it stops changing", () => { atom.config.set('foo.bar.baz', 42) - advanceClock(50) expect(savedSettings.length).toBe(0) atom.config.set('foo.bar.baz', 43) - advanceClock(50) expect(savedSettings.length).toBe(0) atom.config.set('foo.bar.baz', 44) - advanceClock(150) + advanceClock(10) expect(savedSettings.length).toBe(1) }) diff --git a/src/application-delegate.js b/src/application-delegate.js index d6417830a..f0be70a11 100644 --- a/src/application-delegate.js +++ b/src/application-delegate.js @@ -5,6 +5,10 @@ const getWindowLoadSettings = require('./get-window-load-settings') module.exports = class ApplicationDelegate { + constructor () { + this.pendingSettingsUpdateCount = 0 + } + getWindowLoadSettings () { return getWindowLoadSettings() } open (params) { @@ -175,13 +179,20 @@ class ApplicationDelegate { return remote.systemPreferences.getUserDefault(key, type) } - setUserSettings (config) { - return ipcHelpers.call('set-user-settings', config) + async setUserSettings (config) { + this.pendingSettingsUpdateCount++ + try { + await ipcHelpers.call('set-user-settings', config) + } finally { + this.pendingSettingsUpdateCount-- + } } onDidChangeUserSettings (callback) { const outerCallback = (event, message, detail) => { - if (message === 'did-change-user-settings') callback(detail) + if (message === 'did-change-user-settings') { + if (this.pendingSettingsUpdateCount === 0) callback(detail) + } } ipcRenderer.on('message', outerCallback) return new Disposable(() => ipcRenderer.removeListener('message', outerCallback)) diff --git a/src/config-file.js b/src/config-file.js index 877091cf1..38912beb1 100644 --- a/src/config-file.js +++ b/src/config-file.js @@ -5,6 +5,7 @@ const {Emitter} = require('event-kit') const {watchPath} = require('./path-watcher') const CSON = require('season') const Path = require('path') +const async = require('async') const EVENT_TYPES = new Set([ 'created', @@ -16,9 +17,26 @@ module.exports = class ConfigFile { constructor (path) { this.path = path - this.requestLoad = _.debounce(() => this.reload(), 100) this.emitter = new Emitter() this.value = {} + this.reloadCallbacks = [] + + // Use a queue to prevent multiple concurrent write to the same file. + const writeQueue = async.queue((data, callback) => + CSON.writeFile(this.path, data, error => { + if (error) { + this.emitter.emit('did-error', dedent ` + Failed to write \`${Path.basename(this.path)}\`. + + ${error.message} + `) + } + callback() + }) + ) + + this.requestLoad = _.debounce(() => this.reload(), 200) + this.requestSave = _.debounce((data) => writeQueue.push(data), 200) } get () { @@ -26,16 +44,10 @@ class ConfigFile { } update (value) { - return new Promise((resolve, reject) => - CSON.writeFile(this.path, value, error => { - if (error) { - reject(error) - } else { - this.value = value - resolve() - } - }) - ) + return new Promise(resolve => { + this.requestSave(value) + this.reloadCallbacks.push(resolve) + }) } async watch (callback) { @@ -80,6 +92,9 @@ class ConfigFile { } else { this.value = data || {} this.emitter.emit('did-change', this.value) + + for (const callback of this.reloadCallbacks) callback() + this.reloadCallbacks.length = 0 } resolve() }) diff --git a/src/config.js b/src/config.js index f6c00400c..a589654c9 100644 --- a/src/config.js +++ b/src/config.js @@ -430,7 +430,7 @@ class Config { this.transactDepth = 0 this.pendingOperations = [] this.legacyScopeAliases = {} - this.requestSave = _.debounce(() => this.save(), 100) + this.requestSave = _.debounce(() => this.save(), 1) } /*