Improve FileStore read/write safety (#491)

Author: satyakigh

src/datastore/FileStore.ts

@@ -19,6 +19,7 @@ export class FileStoreFactory implements DataStoreFactory {
 
     private readonly metricsInterval: NodeJS.Timeout;
     private readonly timeout: NodeJS.Timeout;
+    private closed = false;
 
     constructor(
         rootDir: string,
@@ -60,12 +61,16 @@ export class FileStoreFactory implements DataStoreFactory {
     }
 
     close(): Promise<void> {
+        if (this.closed) return Promise.resolve();
+        this.closed = true;
         clearTimeout(this.timeout);
         clearInterval(this.metricsInterval);
         return Promise.resolve();
     }
 
     private emitMetrics(): void {
+        if (this.closed) return;
+
         this.telemetry.histogram('version', VersionNumber);
         this.telemetry.histogram('env.entries', this.stores.size);
 
@@ -84,6 +89,8 @@ export class FileStoreFactory implements DataStoreFactory {
     }
 
     private cleanupOldVersions(): void {
+        if (this.closed || !existsSync(this.fileDbRoot)) return;
+
         const entries = readdirSync(this.fileDbRoot, { withFileTypes: true });
         for (const entry of entries) {
             try {

src/datastore/file/EncryptedFileStore.ts

@@ -1,5 +1,5 @@
-import { existsSync, readFileSync, statSync, writeFileSync } from 'fs'; // eslint-disable-line no-restricted-imports -- files being checked
-import { writeFile } from 'fs/promises';
+import { existsSync, readFileSync, renameSync, statSync, writeFileSync } from 'fs'; // eslint-disable-line no-restricted-imports -- files being checked
+import { rename, writeFile } from 'fs/promises';
 import { join } from 'path';
 import { Logger } from 'pino';
 import { lock, LockOptions, lockSync } from 'proper-lockfile';
@@ -28,18 +28,15 @@ export class EncryptedFileStore implements DataStore {
         this.telemetry = TelemetryService.instance.get(`FileStore.${name}`);
 
         if (existsSync(this.file)) {
+            const release = lockSync(this.file, LOCK_OPTIONS_SYNC);
             try {
                 this.content = this.readFile();
             } catch (error) {
                 this.log.error(error, 'Failed to decrypt file store, recreating store');
                 this.telemetry.count('filestore.recreate', 1);
-
-                const release = lockSync(this.file, LOCK_OPTIONS_SYNC);
-                try {
-                    this.saveSync();
-                } finally {
-                    release();
-                }
+                this.saveSync();
+            } finally {
+                release();
             }
         } else {
             this.saveSync();
@@ -113,11 +110,15 @@ export class EncryptedFileStore implements DataStore {
     }
 
     private saveSync() {
-        writeFileSync(this.file, encrypt(this.KEY, JSON.stringify(this.content)));
+        const tmp = `${this.file}.${process.pid}.tmp`;
+        writeFileSync(tmp, encrypt(this.KEY, JSON.stringify(this.content)));
+        renameSync(tmp, this.file);
     }
 
     private async save() {
-        await writeFile(this.file, encrypt(this.KEY, JSON.stringify(this.content)));
+        const tmp = `${this.file}.${process.pid}.tmp`;
+        await writeFile(tmp, encrypt(this.KEY, JSON.stringify(this.content)));
+        await rename(tmp, this.file);
     }
 }