From 7941664a59f90a1b510b13d37062b90210da3b3c Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 15 Feb 2021 10:16:12 -0600 Subject: [PATCH] Lock lowdb storage file to avoid dirty data collisions (#273) * Lock lowdb storage file to avoid dirty data collisions * Retry lock acquire rather than immediately fail * Add proper-lockfile types to dev dependencies * remove proper-lockfile from jslib. This package is incompatible with Browser implementations. * await lock on create --- src/services/lowdbStorage.service.ts | 86 +++++++++++++++++----------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/src/services/lowdbStorage.service.ts b/src/services/lowdbStorage.service.ts index aa69717a198..03890b8bd26 100644 --- a/src/services/lowdbStorage.service.ts +++ b/src/services/lowdbStorage.service.ts @@ -10,34 +10,40 @@ import { NodeUtils } from '../misc/nodeUtils'; import { Utils } from '../misc/utils'; export class LowdbStorageService implements StorageService { + protected dataFilePath: string; private db: lowdb.LowdbSync; private defaults: any; - private dataFilePath: string; - constructor(private logService: LogService, defaults?: any, dir?: string, private allowCache = false) { + constructor(protected logService: LogService, defaults?: any, private dir?: string, private allowCache = false) { this.defaults = defaults; + } + async init() { this.logService.info('Initializing lowdb storage service.'); let adapter: lowdb.AdapterSync; - if (Utils.isNode && dir != null) { - if (!fs.existsSync(dir)) { - this.logService.warning(`Could not find dir, "${dir}"; creating it instead.`); - NodeUtils.mkdirpSync(dir, '700'); - this.logService.info(`Created dir "${dir}".`); - } - this.dataFilePath = path.join(dir, 'data.json'); - if (!fs.existsSync(this.dataFilePath)) { - this.logService.warning(`Could not find data file, "${this.dataFilePath}"; creating it instead.`); - fs.writeFileSync(this.dataFilePath, '', { mode: 0o600 }); - fs.chmodSync(this.dataFilePath, 0o600); - this.logService.info(`Created data file "${this.dataFilePath}" with chmod 600.`); + if (Utils.isNode && this.dir != null) { + if (!fs.existsSync(this.dir)) { + this.logService.warning(`Could not find dir, "${this.dir}"; creating it instead.`); + NodeUtils.mkdirpSync(this.dir, '700'); + this.logService.info(`Created dir "${this.dir}".`); } + this.dataFilePath = path.join(this.dir, 'data.json'); + await this.lockDbFile(() => { + if (!fs.existsSync(this.dataFilePath)) { + this.logService.warning(`Could not find data file, "${this.dataFilePath}"; creating it instead.`); + fs.writeFileSync(this.dataFilePath, '', { mode: 0o600 }); + fs.chmodSync(this.dataFilePath, 0o600); + this.logService.info(`Created data file "${this.dataFilePath}" with chmod 600.`); + } else { + this.logService.info(`db file "${this.dataFilePath} already exists"; using existing db`); + } + }); adapter = new FileSync(this.dataFilePath); } try { this.logService.info('Attempting to create lowdb storage adapter.'); this.db = lowdb(adapter); - this.logService.info('Successfuly created lowdb storage adapter.'); + this.logService.info('Successfully created lowdb storage adapter.'); } catch (e) { if (e instanceof SyntaxError) { this.logService.warning(`Error creating lowdb storage adapter, "${e.message}"; emptying data file.`); @@ -48,36 +54,50 @@ export class LowdbStorageService implements StorageService { throw e; } } - } - init() { if (this.defaults != null) { - this.logService.info('Writing defaults.'); - this.readForNoCache(); - this.db.defaults(this.defaults).write(); - this.logService.info('Successfully wrote defaults to db.'); + this.lockDbFile(() => { + this.logService.info('Writing defaults.'); + this.readForNoCache(); + this.db.defaults(this.defaults).write(); + this.logService.info('Successfully wrote defaults to db.'); + }); } } get(key: string): Promise { - this.readForNoCache(); - const val = this.db.get(key).value(); - if (val == null) { - return Promise.resolve(null); - } - return Promise.resolve(val as T); + return this.lockDbFile(() => { + this.readForNoCache(); + const val = this.db.get(key).value(); + this.logService.debug(`Successfully read ${key} from db`); + if (val == null) { + return null; + } + return val as T; + }); } save(key: string, obj: any): Promise { - this.readForNoCache(); - this.db.set(key, obj).write(); - return Promise.resolve(); + return this.lockDbFile(() => { + this.readForNoCache(); + this.db.set(key, obj).write(); + this.logService.debug(`Successfully wrote ${key} to db`); + return; + }); } remove(key: string): Promise { - this.readForNoCache(); - this.db.unset(key).write(); - return Promise.resolve(); + return this.lockDbFile(() => { + this.readForNoCache(); + this.db.unset(key).write(); + this.logService.debug(`Successfully removed ${key} from db`); + return; + }); + } + + protected async lockDbFile(action: () => T): Promise { + // Lock methods implemented in clients + return Promise.resolve(action()); } private readForNoCache() {