refactor: made _collections thread-safe

Author: Mayo66

Penumbra/Collections/Manager/CollectionStorage.cs

@@ -71,6 +71,8 @@ public void Delete(ModCollection collection)
     [
         ModCollection.Empty,
     ];
+    
+    private readonly Lock _collectionsLock = new();
 
     /// <remarks> A list of all collections ever created still existing by their local id. </remarks>
     private readonly ConcurrentDictionary<LocalCollectionId, ModCollection>
@@ -79,8 +81,11 @@ private readonly ConcurrentDictionary<LocalCollectionId, ModCollection>
 
     public readonly ModCollection DefaultNamed;
 
-    /// <remarks> Incremented by 1 because the empty collection gets Zero. </remarks>
-    public LocalCollectionId CurrentCollectionId { get; private set; } = LocalCollectionId.Zero + 1;
+    /// <remarks> Starts at 1 because the empty collection gets Zero. </remarks>
+    private int _currentCollectionIdValue = 1;
+    
+    /// <remarks> Starts at 1 because the empty collection gets Zero. </remarks>
+    public LocalCollectionId CurrentCollectionId => new(_currentCollectionIdValue);
 
     /// <summary> Default enumeration skips the empty collection. </summary>
     public IEnumerator<ModCollection> GetEnumerator()
@@ -162,8 +167,12 @@ public bool AddCollection(string name, ModCollection? duplicate)
         if (name.Length == 0)
             return false;
 
-        var newCollection = Create(name, _collections.Count, duplicate);
-        _collections.Add(newCollection);
+        ModCollection newCollection;
+        lock (_collectionsLock)
+        {
+            newCollection = Create(name, _collections.Count, duplicate);
+            _collections.Add(newCollection);
+        }
         _saveService.ImmediateSave(new ModCollectionSave(_modStorage, newCollection));
         Penumbra.Messager.NotificationMessage($"Created new collection {newCollection.Identity.AnonymizedName}.", NotificationType.Success, false);
         _communicator.CollectionChange.Invoke(CollectionType.Inactive, null, newCollection, string.Empty);
@@ -189,10 +198,13 @@ public bool RemoveCollection(ModCollection collection)
 
         Delete(collection);
         _saveService.ImmediateDelete(new ModCollectionSave(_modStorage, collection));
-        _collections.RemoveAt(collection.Identity.Index);
-        // Update indices.
-        for (var i = collection.Identity.Index; i < Count; ++i)
-            _collections[i].Identity.Index = i;
+        lock (_collectionsLock)
+        {
+            _collections.RemoveAt(collection.Identity.Index);
+            // Update indices.
+            for (var i = collection.Identity.Index; i < Count; ++i)
+                _collections[i].Identity.Index = i;
+        }
 
         Penumbra.Messager.NotificationMessage($"Deleted collection {collection.Identity.AnonymizedName}.", NotificationType.Success, false);
         _communicator.CollectionChange.Invoke(CollectionType.Inactive, collection, null, string.Empty);
@@ -316,38 +328,41 @@ private ModCollection SetDefaultNamedCollection()
     /// <summary> Move all settings in all collections to unused settings. </summary>
     private void OnModDiscoveryStarted()
     {
-        foreach (var collection in this)
+        var snapshot = GetModSnapShot();
+        foreach (var collection in snapshot)
             collection.Settings.PrepareModDiscovery(_modStorage);
     }
 
     /// <summary> Restore all settings in all collections to mods. </summary>
     private void OnModDiscoveryFinished()
     {
+        var snapshot = GetModSnapShot();
         // Re-apply all mod settings.
-        foreach (var collection in this)
+        foreach (var collection in snapshot)
             collection.Settings.ApplyModSettings(collection, _saveService, _modStorage);
     }
 
     /// <summary> Add or remove a mod from all collections, or re-save all collections where the mod has settings. </summary>
     private void OnModPathChange(ModPathChangeType type, Mod mod, DirectoryInfo? oldDirectory,
         DirectoryInfo? newDirectory)
     {
+        var snapshot = GetModSnapShot();
         switch (type)
         {
             case ModPathChangeType.Added:
-                foreach (var collection in this)
+                foreach (var collection in snapshot)
                     collection.Settings.AddMod(mod);
                 break;
             case ModPathChangeType.Deleted:
-                foreach (var collection in this)
+                foreach (var collection in snapshot)
                     collection.Settings.RemoveMod(mod);
                 break;
             case ModPathChangeType.Moved:
-                foreach (var collection in this.Where(collection => collection.GetOwnSettings(mod.Index) != null))
+                foreach (var collection in snapshot.Where(collection => collection.GetOwnSettings(mod.Index) != null))
                     _saveService.QueueSave(new ModCollectionSave(_modStorage, collection));
                 break;
             case ModPathChangeType.Reloaded:
-                foreach (var collection in this)
+                foreach (var collection in snapshot)
                 {
                     if (collection.GetOwnSettings(mod.Index)?.Settings.FixAll(mod) ?? false)
                         _saveService.QueueSave(new ModCollectionSave(_modStorage, collection));
@@ -365,8 +380,9 @@ private void OnModOptionChange(ModOptionChangeType type, Mod mod, IModGroup? gro
         type.HandlingInfo(out var requiresSaving, out _, out _);
         if (!requiresSaving)
             return;
-
-        foreach (var collection in this)
+        
+        var snapshot = GetModSnapShot();
+        foreach (var collection in snapshot)
         {
             if (collection.GetOwnSettings(mod.Index)?.HandleChanges(type, mod, group, option, movedToIdx) ?? false)
                 _saveService.QueueSave(new ModCollectionSave(_modStorage, collection));
@@ -380,11 +396,22 @@ private void OnModFileChanged(Mod mod, FileRegistry file)
         if (file.CurrentUsage == 0)
             return;
 
-        foreach (var collection in this)
+        var snapshot = GetModSnapShot();
+        foreach (var collection in snapshot)
         {
             var (settings, _) = collection.GetActualSettings(mod.Index);
             if (settings is { Enabled: true })
                 collection.Counters.IncrementChange();
         }
     }
+    
+    private ModCollection[] GetModSnapShot()
+    {
+        ModCollection[] snapshot;
+        lock (_collectionsLock)
+        {
+            snapshot = _collections.Skip(1).ToArray();
+        }
+        return snapshot;
+    }
 }