Fix SecureSettings cache race (#4620)

Replace the SecureSettings cache with ConcurrentDictionary so parallel package manager initialization cannot corrupt shared state.

Add regression coverage for concurrent cache misses in SecureSettings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: mamoreau-devolutions

src/UniGetUI.Core.SecureSettings/SecureSettings.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using System.Diagnostics;
 using UniGetUI.Core.Data;
 using UniGetUI.Core.Tools;
@@ -38,7 +39,7 @@ public static string ResolveKey(K key)
         };
     }
 
-    private static readonly Dictionary<string, bool> _cache = new();
+    private static readonly ConcurrentDictionary<string, bool> _cache = new();
 
     public static class Args
     {
@@ -49,31 +50,13 @@ public static class Args
     public static bool Get(K key)
     {
         string purifiedSetting = CoreTools.MakeValidFileName(ResolveKey(key));
-        if (_cache.TryGetValue(purifiedSetting, out var value))
-        {
-            return value;
-        }
-
-        string purifiedUser = CoreTools.MakeValidFileName(Environment.UserName);
-
-        var settingsLocation = Path.Join(GetSecureSettingsRoot(), purifiedUser);
-        var settingFile = Path.Join(settingsLocation, purifiedSetting);
-
-        if (!Directory.Exists(settingsLocation))
-        {
-            _cache[purifiedSetting] = false;
-            return false;
-        }
-
-        bool exists = File.Exists(settingFile);
-        _cache[purifiedSetting] = exists;
-        return exists;
+        return _cache.GetOrAdd(purifiedSetting, ResolveSettingValue);
     }
 
     public static async Task<bool> TrySet(K key, bool enabled)
     {
         string purifiedSetting = CoreTools.MakeValidFileName(ResolveKey(key));
-        _cache.Remove(purifiedSetting);
+        _cache.TryRemove(purifiedSetting, out _);
 
         string purifiedUser = CoreTools.MakeValidFileName(Environment.UserName);
 
@@ -107,7 +90,7 @@ public static int ApplyForUser(string username, string setting, bool enable)
         try
         {
             string purifiedSetting = CoreTools.MakeValidFileName(setting);
-            _cache.Remove(purifiedSetting);
+            _cache.TryRemove(purifiedSetting, out _);
 
             string purifiedUser = CoreTools.MakeValidFileName(username);
 
@@ -158,4 +141,17 @@ private static string GetSecureSettingsRoot()
 
         return Path.Join(CoreData.UniGetUIDataDirectory, "SecureSettings");
     }
+
+    private static bool ResolveSettingValue(string purifiedSetting)
+    {
+        string purifiedUser = CoreTools.MakeValidFileName(Environment.UserName);
+        var settingsLocation = Path.Join(GetSecureSettingsRoot(), purifiedUser);
+        if (!Directory.Exists(settingsLocation))
+        {
+            return false;
+        }
+
+        var settingFile = Path.Join(settingsLocation, purifiedSetting);
+        return File.Exists(settingFile);
+    }
 }

src/UniGetUI.Core.Settings.Tests/SecureSettingsTests.cs

@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using System.Reflection;
 using UniGetUI.Core.Tools;
 using SecureSettingsStore = UniGetUI.Core.SettingsEngine.SecureSettings.SecureSettings;
@@ -87,6 +88,40 @@ public void Get_RefreshesCachedValueAfterApplyForUserWrites()
         Assert.False(SecureSettingsStore.Get(SecureSettingsStore.K.AllowCLIArguments));
     }
 
+    [Fact]
+    public async Task Get_AllowsConcurrentCacheMisses()
+    {
+        string username = Environment.UserName;
+        string setting = SecureSettingsStore.ResolveKey(
+            SecureSettingsStore.K.AllowCustomManagerPaths
+        );
+        Assert.Equal(0, SecureSettingsStore.ApplyForUser(username, setting, true));
+
+        for (int iteration = 0; iteration < 25; iteration++)
+        {
+            ClearSecureSettingsCache();
+            using ManualResetEventSlim startGate = new(false);
+
+            Task<bool>[] tasks = Enumerable
+                .Range(0, 64)
+                .Select(_ =>
+                    Task.Run(() =>
+                    {
+                        startGate.Wait();
+                        return SecureSettingsStore.Get(
+                            SecureSettingsStore.K.AllowCustomManagerPaths
+                        );
+                    })
+                )
+                .ToArray();
+
+            startGate.Set();
+            bool[] results = await Task.WhenAll(tasks);
+
+            Assert.All(results, Assert.True);
+        }
+    }
+
     private string GetCurrentUserSettingsDirectory() =>
         Path.Combine(_testRoot, CoreTools.MakeValidFileName(Environment.UserName));
 
@@ -97,15 +132,16 @@ private string GetSettingsFilePath(string username, string setting) =>
             CoreTools.MakeValidFileName(setting)
         );
 
-    private static void ClearSecureSettingsCache()
+    private static ConcurrentDictionary<string, bool> GetCache()
     {
         FieldInfo? cacheField = typeof(SecureSettingsStore).GetField(
             "_cache",
             BindingFlags.NonPublic | BindingFlags.Static
         );
         Assert.NotNull(cacheField);
 
-        var cache = Assert.IsType<Dictionary<string, bool>>(cacheField.GetValue(null));
-        cache.Clear();
+        return Assert.IsType<ConcurrentDictionary<string, bool>>(cacheField.GetValue(null));
     }
+
+    private static void ClearSecureSettingsCache() => GetCache().Clear();
 }