Kept overlapping settings operations from freezing the Document Analysis assistant (#970)

Co-authored-by: Thorsten Sommer <SommerEngineering@users.noreply.github.com>

app/MindWork AI Studio/Settings/SettingsManager.cs

@@ -34,6 +34,22 @@ public sealed class SettingsManager
     private readonly ILogger<SettingsManager> logger;
     private readonly RustService rustService;
 
+    /// <summary>
+    /// Lets only one operation at a time touch the settings files.
+    /// </summary>
+    /// <remarks>
+    /// Reading takes this as well as writing does, for two reasons. A read migrates and backs up
+    /// what it found, so it writes the very files a store writes. And it re-evaluates whether
+    /// writes are blocked at all, starting out by clearing that block: a store slipping through
+    /// that moment would overwrite the settings the block exists to protect.<br/><br/>
+    /// What this does not do is guard the settings themselves. It guards the files: what one store
+    /// writes, the next one no longer has to fear. The configuration data behind them stays open to
+    /// everybody, and a store serializes it while the rest of the app goes on editing it -- a list
+    /// growing mid-serialization still throws. Whoever wants that answered needs one of their own;
+    /// this lock is not it.
+    /// </remarks>
+    private readonly SemaphoreSlim settingsFileSemaphore = new(1, 1);
+
     /// <summary>
     /// The settings manager.
     /// </summary>
@@ -103,6 +119,19 @@ public sealed class SettingsManager
     /// </summary>
     /// <returns>A (migrated) settings snapshot, or null if it could not be read.</returns>
     public async Task<Data?> TryReadSettingsSnapshot()
+    {
+        await this.settingsFileSemaphore.WaitAsync();
+        try
+        {
+            return await this.ReadSettingsSnapshot();
+        }
+        finally
+        {
+            this.settingsFileSemaphore.Release();
+        }
+    }
+
+    private async Task<Data?> ReadSettingsSnapshot()
     {
         this.SettingsWriteBlockReason = SettingsWriteBlockReason.NONE;
         if(!this.IsSetUp)
@@ -294,41 +323,72 @@ public sealed class SettingsManager
     /// </summary>
     public async Task StoreSettings()
     {
-        if(!this.IsSetUp)
+        await this.settingsFileSemaphore.WaitAsync();
+        try
         {
-            this.logger.LogWarning("Cannot store settings, because the configuration is not set up yet.");
-            return;
-        }
+            if(!this.IsSetUp)
+            {
+                this.logger.LogWarning("Cannot store settings, because the configuration is not set up yet.");
+                return;
+            }
 
-        if(this.SettingsWriteBlocked)
+            if(this.SettingsWriteBlocked)
+            {
+                this.logger.LogWarning($"Cannot store settings, because settings writes are blocked. Reason: '{this.SettingsWriteBlockReason}'.");
+                return;
+            }
+
+            var settingsJson = JsonSerializer.Serialize(this.ConfigurationData, JSON_OPTIONS);
+            var settingsPath = Path.Combine(ConfigDirectory!, SETTINGS_FILENAME);
+            await this.StoreSerializedSettings(settingsJson, settingsPath);
+            await this.StoreSerializedVersionBackup(this.ConfigurationData.Version, settingsJson);
+        }
+        finally
         {
-            this.logger.LogWarning($"Cannot store settings, because settings writes are blocked. Reason: '{this.SettingsWriteBlockReason}'.");
-            return;
+            this.settingsFileSemaphore.Release();
         }
-
-        var settingsPath = Path.Combine(ConfigDirectory!, SETTINGS_FILENAME);
-        await this.StoreSettingsSnapshot(this.ConfigurationData, settingsPath);
-        await this.StoreCurrentVersionBackup(this.ConfigurationData);
     }
 
     private static string GetBackupSettingsFilename(Version version) => $"settings.{version.ToString().ToLowerInvariant()}.json";
 
     private static string GetBackupSettingsPath(Version version) => Path.Combine(ConfigDirectory!, GetBackupSettingsFilename(version));
 
-    private async Task StoreCurrentVersionBackup(Data settingsData)
+    private Task StoreCurrentVersionBackup(Data settingsData) =>
+        this.StoreSerializedVersionBackup(settingsData.Version, JsonSerializer.Serialize(settingsData, JSON_OPTIONS));
+
+    /// <summary>
+    /// Writes the backup file from settings which were serialized already.
+    /// </summary>
+    /// <remarks>
+    /// The store hands the same JSON to this method and to the one writing the settings file, so
+    /// that both files say the same thing. Serializing twice cannot promise that: the configuration
+    /// data may well have changed in between, and the backup would then describe a state the
+    /// settings file never had.
+    /// </remarks>
+    /// <param name="settingsVersion">The version the serialized settings carry.</param>
+    /// <param name="settingsJson">The serialized settings.</param>
+    private async Task StoreSerializedVersionBackup(Version settingsVersion, string settingsJson)
     {
-        if(settingsData.Version != CURRENT_SETTINGS_VERSION)
+        if(settingsVersion != CURRENT_SETTINGS_VERSION)
         {
-            this.logger.LogWarning($"Skipping settings backup because the settings version '{settingsData.Version}' is not the current version '{CURRENT_SETTINGS_VERSION}'.");
+            this.logger.LogWarning($"Skipping settings backup because the settings version '{settingsVersion}' is not the current version '{CURRENT_SETTINGS_VERSION}'.");
             return;
         }
 
         var backupSettingsPath = GetBackupSettingsPath(CURRENT_SETTINGS_VERSION);
-        await this.StoreSettingsSnapshot(settingsData, backupSettingsPath);
+        await this.StoreSerializedSettings(settingsJson, backupSettingsPath);
         this.logger.LogInformation($"Stored the settings backup file '{backupSettingsPath}'.");
     }
 
-    private async Task StoreSettingsSnapshot(Data settingsData, string settingsPath)
+    private Task StoreSettingsSnapshot(Data settingsData, string settingsPath) =>
+        this.StoreSerializedSettings(JsonSerializer.Serialize(settingsData, JSON_OPTIONS), settingsPath);
+
+    /// <summary>
+    /// Writes settings which were serialized already to the given path.
+    /// </summary>
+    /// <param name="settingsJson">The serialized settings.</param>
+    /// <param name="settingsPath">The file to write them to.</param>
+    private async Task StoreSerializedSettings(string settingsJson, string settingsPath)
     {
         if(!Directory.Exists(ConfigDirectory))
         {
@@ -336,8 +396,6 @@ public sealed class SettingsManager
             Directory.CreateDirectory(ConfigDirectory!);
         }
 
-        var settingsJson = JsonSerializer.Serialize(settingsData, JSON_OPTIONS);
-
         //
         // We write the new settings next to the previous ones and replace them afterwards, so that
         // no crash can leave a half-written settings file behind. The temporary file has to live in