Safer profile saving handling (#1119)

Handles this more safely by making each call to this wait for a lock to
be released first, rather than throwing an error that might not be
handled everywhere this method is called.

Co-authored-by: Chomp <27521899+chompDev@users.noreply.github.com>

Author: chompDev

project/package.json

@@ -34,6 +34,7 @@
         "gen:productionquests": "tsx ./src/tools/ProductionQuestsGen/ProductionQuestsGenProgram.ts"
     },
     "dependencies": {
+        "async-mutex": "^0.5.0",
         "atomically": "2.0.3",
         "fs-extra": "11.2.0",
         "i18n": "0.15.1",

project/src/servers/SaveServer.ts

@@ -9,13 +9,14 @@ import { FileSystem } from "@spt/utils/FileSystem";
 import { HashUtil } from "@spt/utils/HashUtil";
 import { JsonUtil } from "@spt/utils/JsonUtil";
 import { Timer } from "@spt/utils/Timer";
+import { Mutex } from "async-mutex";
 import { inject, injectAll, injectable } from "tsyringe";
 
 @injectable()
 export class SaveServer {
     protected profileFilepath = "user/profiles/";
     protected profiles: Map<string, ISptProfile> = new Map();
-    protected profilesBeingSaved: Set<string> = new Set();
+    protected profilesBeingSavedMutex: Map<string, Mutex> = new Map();
     protected onBeforeSaveCallbacks: Map<string, (profile: ISptProfile) => Promise<ISptProfile>> = new Map();
     protected saveSHA1: { [key: string]: string } = {};
 
@@ -188,43 +189,48 @@ export class SaveServer {
      * @returns A promise that resolves when saving is completed.
      */
     public async saveProfile(sessionID: string): Promise<void> {
-        if (!this.profiles.get(sessionID)) {
-            throw new Error(`Profile ${sessionID} does not exist! Unable to save this profile!`);
-        }
-
-        if (this.profilesBeingSaved.has(sessionID)) {
-            throw new Error(`Profile ${sessionID} is already being saved!`);
-        }
+        // Get the current mutex if it exists, create a new one if it doesn't for this profile
+        const mutex = this.profilesBeingSavedMutex.get(sessionID) || new Mutex();
+        this.profilesBeingSavedMutex.set(sessionID, mutex);
 
-        this.profilesBeingSaved.add(sessionID);
+        const release = await mutex.acquire();
 
-        const filePath = `${this.profileFilepath}${sessionID}.json`;
+        try {
+            if (!this.profiles.get(sessionID)) {
+                throw new Error(`Profile ${sessionID} does not exist! Unable to save this profile!`);
+            }
 
-        // Run pre-save callbacks before we save into json
-        for (const [id, callback] of this.onBeforeSaveCallbacks) {
-            const previous = this.profiles.get(sessionID) as ISptProfile; // Cast as ISptProfile here since there should be no reason we're getting an undefined profile
-            try {
-                this.profiles.set(sessionID, await callback(this.profiles.get(sessionID) as ISptProfile)); // Cast as ISptProfile here since there should be no reason we're getting an undefined profile
-            } catch (error) {
-                this.logger.error(this.localisationService.getText("profile_save_callback_error", { callback, error }));
-                this.profiles.set(sessionID, previous);
+            const filePath = `${this.profileFilepath}${sessionID}.json`;
+
+            // Run pre-save callbacks before we save into json
+            for (const [id, callback] of this.onBeforeSaveCallbacks) {
+                const previous = this.profiles.get(sessionID) as ISptProfile; // Cast as ISptProfile here since there should be no reason we're getting an undefined profile
+                try {
+                    this.profiles.set(sessionID, await callback(this.profiles.get(sessionID) as ISptProfile)); // Cast as ISptProfile here since there should be no reason we're getting an undefined profile
+                } catch (error) {
+                    this.logger.error(
+                        this.localisationService.getText("profile_save_callback_error", { callback, error }),
+                    );
+                    this.profiles.set(sessionID, previous);
+                }
             }
-        }
 
-        const jsonProfile = this.jsonUtil.serialize(
-            this.profiles.get(sessionID),
-            !this.configServer.getConfig<ICoreConfig>(ConfigTypes.CORE).features.compressProfile,
-        );
+            const jsonProfile = this.jsonUtil.serialize(
+                this.profiles.get(sessionID),
+                !this.configServer.getConfig<ICoreConfig>(ConfigTypes.CORE).features.compressProfile,
+            );
 
-        const sha1 = await this.hashUtil.generateSha1ForDataAsync(jsonProfile);
+            const sha1 = await this.hashUtil.generateSha1ForDataAsync(jsonProfile);
 
-        if (typeof this.saveSHA1[sessionID] !== "string" || this.saveSHA1[sessionID] !== sha1) {
-            this.saveSHA1[sessionID] = sha1;
-            // save profile to disk
-            await this.fileSystem.write(filePath, jsonProfile);
+            if (typeof this.saveSHA1[sessionID] !== "string" || this.saveSHA1[sessionID] !== sha1) {
+                this.saveSHA1[sessionID] = sha1;
+                // save profile to disk
+                await this.fileSystem.write(filePath, jsonProfile);
+            }
+        } finally {
+            // Release the current lock
+            release();
         }
-
-        this.profilesBeingSaved.delete(sessionID);
     }
 
     /**
@@ -237,6 +243,16 @@ export class SaveServer {
 
         this.profiles.delete(sessionID);
 
+        const pendingMutex = this.profilesBeingSavedMutex.get(sessionID);
+
+        if (pendingMutex) {
+            // If the profile already has a mutex assigned, cancel all pending locks and release.
+            pendingMutex.cancel();
+            pendingMutex.release();
+
+            this.profilesBeingSavedMutex.delete(sessionID);
+        }
+
         await this.fileSystem.remove(file);
 
         return !(await this.fileSystem.exists(file));