Fixes concurrency issues with Application.Toplevels

Examples/UICatalog/Scenarios/VkeyPacketSimulator.cs

@@ -3,7 +3,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Terminal.Gui;
-using Terminal.Gui.ConsoleDrivers;
 
 namespace UICatalog.Scenarios;
 

Terminal.Gui/Application/Application.Run.cs

@@ -150,10 +150,10 @@ public static RunState Begin (Toplevel toplevel)
                 }
             }
 
-            if (TopLevels.FindDuplicates (new ToplevelEqualityComparer ()).Count > 0)
-            {
-                throw new ArgumentException ("There are duplicates Toplevel IDs");
-            }
+            //if (TopLevels.FindDuplicates (new ToplevelEqualityComparer ()).Count > 0)
+            //{
+            //    throw new ArgumentException ("There are duplicates Toplevel IDs");
+            //}
         }
 
         if (Top is null)
@@ -588,25 +588,23 @@ public static void End (RunState runState)
 
         // End the RunState.Toplevel
         // First, take it off the Toplevel Stack
-        if (TopLevels.Count > 0)
+        if (TopLevels.TryPop (out Toplevel? topOfStack))
         {
-            if (TopLevels.Peek () != runState.Toplevel)
+            if (topOfStack != runState.Toplevel)
             {
                 // If the top of the stack is not the RunState.Toplevel then
                 // this call to End is not balanced with the call to Begin that started the RunState
                 throw new ArgumentException ("End must be balanced with calls to Begin");
             }
-
-            TopLevels.Pop ();
         }
 
         // Notify that it is closing
         runState.Toplevel?.OnClosed (runState.Toplevel);
 
-        if (TopLevels.Count > 0)
+        if (TopLevels.TryPeek (out Toplevel? newTop))
         {
-            Top = TopLevels.Peek ();
-            Top.SetNeedsDraw ();
+            Top = newTop;
+            Top?.SetNeedsDraw ();
         }
 
         if (runState.Toplevel is { HasFocus: true })

Terminal.Gui/Application/Application.Toplevel.cs

@@ -1,12 +1,26 @@
 #nullable enable
+using System.Collections.Concurrent;
+
 namespace Terminal.Gui;
 
 public static partial class Application // Toplevel handling
 {
     // BUGBUG: Technically, this is not the full lst of TopLevels. There be dragons here, e.g. see how Toplevel.Id is used. What
 
+    private static readonly ConcurrentStack<Toplevel> _topLevels = new ();
+    private static readonly object _topLevelsLock = new ();
+
     /// <summary>Holds the stack of TopLevel views.</summary>
-    internal static Stack<Toplevel> TopLevels { get; } = new ();
+    internal static ConcurrentStack<Toplevel> TopLevels
+    {
+        get
+        {
+            lock (_topLevelsLock)
+            {
+                return _topLevels;
+            }
+        }
+    }
 
     /// <summary>The <see cref="Toplevel"/> that is currently active.</summary>
     /// <value>The top.</value>

Terminal.Gui/Configuration/ConfigurationManager.cs

@@ -8,7 +8,6 @@
 using System.Text.Encodings.Web;
 using System.Text.Json;
 using System.Text.Json.Serialization;
-using Terminal.Gui.Configuration;
 
 namespace Terminal.Gui;
 
@@ -150,7 +149,7 @@ internal static bool IsInitialized ()
     private static ImmutableSortedDictionary<string, ConfigProperty>? _uninitializedConfigPropertiesCache;
 
 #pragma warning disable IDE1006 // Naming Styles
-    private static readonly object __uninitializedConfigPropertiesCacheCacheLock = new ();
+    private static readonly object _uninitializedConfigPropertiesCacheCacheLock = new ();
 #pragma warning restore IDE1006 // Naming Styles
 
     /// <summary>
@@ -181,7 +180,7 @@ internal static void Initialize ()
         ConfigProperty.Initialize ();
 
         // Cache all configuration properties
-        lock (__uninitializedConfigPropertiesCacheCacheLock)
+        lock (_uninitializedConfigPropertiesCacheCacheLock)
         {
             // _allConfigProperties: for ordered, iterable access (LINQ-friendly)
             // _frozenConfigPropertyCache: for high-speed key lookup (frozen)
@@ -777,15 +776,17 @@ public static string GetHardCodedConfig ()
             return _uninitializedConfigPropertiesCache;
         }
 
-        // Filter properties by scope using the cached ScopeType property instead of reflection
-        IEnumerable<KeyValuePair<string, ConfigProperty>>? filtered = _uninitializedConfigPropertiesCache?.Where (cp => cp.Value.ScopeType == scopeType);
-
-        Debug.Assert (filtered is { });
+        lock (_uninitializedConfigPropertiesCacheCacheLock)
+        {
+            // Filter properties by scope using the cached ScopeType property instead of reflection
+            IEnumerable<KeyValuePair<string, ConfigProperty>>? filtered = _uninitializedConfigPropertiesCache?.Where (cp => cp.Value.ScopeType == scopeType);
 
-        IEnumerable<KeyValuePair<string, ConfigProperty>> configPropertiesByScope = filtered as KeyValuePair<string, ConfigProperty> [] ?? filtered.ToArray ();
-        Debug.Assert (configPropertiesByScope.All (v => !v.Value.HasValue));
+            Debug.Assert (filtered is { });
 
-        return configPropertiesByScope;
+            IEnumerable<KeyValuePair<string, ConfigProperty>> configPropertiesByScope = filtered as KeyValuePair<string, ConfigProperty> [] ?? filtered.ToArray ();
+            Debug.Assert (configPropertiesByScope.All (v => !v.Value.HasValue));
+            return configPropertiesByScope;
+        }
     }
 
     /// <summary>

Terminal.Gui/Configuration/DeepCloner.cs

@@ -285,21 +285,32 @@ private static object CloneDictionary (object source, ConcurrentDictionary<objec
         IDictionary tempDict = CreateDictionaryInstance (dictType, comparer);
         visited.TryAdd (source, tempDict);
 
-        // Clone all key-value pairs
-        foreach (object? key in sourceDict.Keys)
-        {
-            object? clonedKey = DeepCloneInternal (key, visited);
-            object? clonedValue = DeepCloneInternal (sourceDict [key], visited);
 
-            if (tempDict.Contains (clonedKey!))
-            {
-                tempDict [clonedKey!] = clonedValue;
-            }
-            else
+        object? lastKey = null;
+        try
+        {
+            // Clone all key-value pairs
+            foreach (object? key in sourceDict.Keys)
             {
-                tempDict.Add (clonedKey!, clonedValue);
+                lastKey = key;
+                object? clonedKey = DeepCloneInternal (key, visited);
+                object? clonedValue = DeepCloneInternal (sourceDict [key], visited);
+
+                if (tempDict.Contains (clonedKey!))
+                {
+                    tempDict [clonedKey!] = clonedValue;
+                }
+                else
+                {
+                    tempDict.Add (clonedKey!, clonedValue);
+                }
             }
         }
+        catch (InvalidOperationException ex)
+        {
+            // Handle cases where the dictionary is modified during enumeration
+            throw new InvalidOperationException ($"Error cloning dictionary ({source}) (last key was \"{lastKey}\"). Ensure the source dictionary is not modified during cloning.", ex);
+        }
 
         // If the original dictionary type has a parameterless constructor, create a new instance
         if (type.GetConstructor (Type.EmptyTypes) != null)

Terminal.Gui/Configuration/SourcesManager.cs

@@ -4,7 +4,7 @@
 using System.Reflection;
 using System.Text.Json;
 
-namespace Terminal.Gui.Configuration;
+namespace Terminal.Gui;
 
 /// <summary>
 ///    Manages the <see cref="ConfigurationManager"/> Sources and provides the API for loading them. Source is a location where a configuration can be stored. Sources are defined in <see cref="ConfigLocations"/>.

Terminal.Gui/Configuration/ThemeManager.cs

@@ -101,6 +101,10 @@ public static ImmutableList<string> GetThemeNames ()
     /// <returns></returns>
     public static string GetCurrentThemeName () { return Theme!; }
 
+    // TODO: Add a lock around Theme and Themes
+    // TODO: For now, this test can't run in parallel with other tests that access Theme or Themes.
+    // TODO: ThemeScopeList_WithThemes_ClonesSuccessfully
+
     /// <summary>
     ///     Gets the Themes dictionary. <see cref="GetThemes"/> is preferred.
     ///     The backing store is <c><see cref="ConfigurationManager.Settings"/> ["Themes"]</c>.

Terminal.Gui/ConsoleDrivers/ConsoleKeyMapping.cs

@@ -1,7 +1,7 @@
 using System.Globalization;
 using System.Runtime.InteropServices;
 
-namespace Terminal.Gui.ConsoleDrivers;
+namespace Terminal.Gui;
 
 // QUESTION: This class combines Windows specific code with cross-platform code. Should this be split into two classes?
 /// <summary>Helper class to handle the scan code and virtual key from a <see cref="ConsoleKey"/>.</summary>

Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs

@@ -4,7 +4,6 @@
 //
 
 using System.Runtime.InteropServices;
-using Terminal.Gui.ConsoleDrivers;
 using Unix.Terminal;
 
 namespace Terminal.Gui;
@@ -729,7 +728,7 @@
                 while (wch2 == Curses.KeyMouse)
                 {
                     // BUGBUG: Fix this nullable issue.
                     Key kea = null;
 
                     ConsoleKeyInfo [] cki =
                     {
@@ -739,7 +738,7 @@
                     };
                     code = 0;
                     // BUGBUG: Fix this nullable issue.
                     HandleEscSeqResponse (ref code, ref k, ref wch2, ref kea, ref cki);
                 }
 
                 return;
@@ -801,7 +800,7 @@
             }
 
             // BUGBUG: Fix this nullable issue.
             Key key = null;
 
             if (code == 0)
             {

Terminal.Gui/ConsoleDrivers/EscSeqUtils/EscSeqUtils.cs

@@ -1,6 +1,5 @@
 #nullable enable
 using System.Globalization;
-using static Terminal.Gui.ConsoleDrivers.ConsoleKeyMapping;
 
 namespace Terminal.Gui;
 
@@ -1159,7 +1158,7 @@ public static ConsoleKeyInfo MapConsoleKeyInfo (ConsoleKeyInfo consoleKeyInfo)
 
                 break;
             default:
-                uint ck = MapKeyCodeToConsoleKey ((KeyCode)consoleKeyInfo.KeyChar, out bool isConsoleKey);
+                uint ck = ConsoleKeyMapping.MapKeyCodeToConsoleKey ((KeyCode)consoleKeyInfo.KeyChar, out bool isConsoleKey);
 
                 if (isConsoleKey)
                 {
@@ -1415,12 +1414,12 @@ internal static KeyCode MapKey (ConsoleKeyInfo keyInfo)
                 if (keyInfo.Modifiers != ConsoleModifiers.Shift)
                 {
                     // If Shift wasn't down we don't need to do anything but return the keyInfo.KeyChar
-                    return MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)keyInfo.KeyChar);
+                    return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)keyInfo.KeyChar);
                 }
 
                 // Strip off Shift - We got here because they KeyChar from Windows is the shifted char (e.g. "Ç")
                 // and passing on Shift would be redundant.
-                return MapToKeyCodeModifiers (keyInfo.Modifiers & ~ConsoleModifiers.Shift, (KeyCode)keyInfo.KeyChar);
+                return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers & ~ConsoleModifiers.Shift, (KeyCode)keyInfo.KeyChar);
         }
 
         // Handle control keys whose VK codes match the related ASCII value (those below ASCII 33) like ESC
@@ -1431,14 +1430,14 @@ internal static KeyCode MapKey (ConsoleKeyInfo keyInfo)
                 return KeyCode.Tab;
             }
 
-            return MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)(uint)keyInfo.Key);
+            return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)(uint)keyInfo.Key);
         }
 
         // Handle control keys (e.g. CursorUp)
         if (keyInfo.Key != ConsoleKey.None
             && Enum.IsDefined (typeof (KeyCode), (uint)keyInfo.Key + (uint)KeyCode.MaxCodePoint))
         {
-            return MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)((uint)keyInfo.Key + (uint)KeyCode.MaxCodePoint));
+            return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)((uint)keyInfo.Key + (uint)KeyCode.MaxCodePoint));
         }
 
         if ((ConsoleKey)keyInfo.KeyChar is >= ConsoleKey.A and <= ConsoleKey.Z)
@@ -1469,7 +1468,7 @@ internal static KeyCode MapKey (ConsoleKeyInfo keyInfo)
                 || keyInfo.Modifiers.HasFlag (ConsoleModifiers.Control))
             {
                 // NetDriver doesn't support Shift-Ctrl/Shift-Alt combos
-                return MapToKeyCodeModifiers (keyInfo.Modifiers & ~ConsoleModifiers.Shift, (KeyCode)keyInfo.Key);
+                return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers & ~ConsoleModifiers.Shift, (KeyCode)keyInfo.Key);
             }
 
             if (keyInfo.Modifiers == ConsoleModifiers.Shift)
@@ -1484,7 +1483,7 @@ internal static KeyCode MapKey (ConsoleKeyInfo keyInfo)
             return (KeyCode)keyInfo.Key;
         }
 
-        return MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)keyInfo.KeyChar);
+        return ConsoleKeyMapping.MapToKeyCodeModifiers (keyInfo.Modifiers, (KeyCode)keyInfo.KeyChar);
     }
 
     private static async Task ProcessButtonClickedAsync ()

Terminal.Gui/ConsoleDrivers/FakeDriver/FakeConsole.cs

@@ -2,8 +2,6 @@
 // FakeConsole.cs: A fake .NET Windows Console API implementation for unit tests.
 //
 
-using Terminal.Gui.ConsoleDrivers;
-
 namespace Terminal.Gui;
 
 #pragma warning disable RCS1138 // Add summary to documentation comment.

Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs

@@ -4,7 +4,6 @@
 
 using System.Diagnostics;
 using System.Runtime.InteropServices;
-using Terminal.Gui.ConsoleDrivers;
 
 // Alias Console to MockConsole so we don't accidentally use Console
 

Terminal.Gui/ConsoleDrivers/V2/WindowsKeyConverter.cs

@@ -1,5 +1,4 @@
 #nullable enable
-using Terminal.Gui.ConsoleDrivers;
 
 namespace Terminal.Gui;
 

Terminal.Gui/ConsoleDrivers/WindowsDriver/WindowsConsole.cs

@@ -2,7 +2,6 @@
 using System.Collections.Concurrent;
 using System.ComponentModel;
 using System.Runtime.InteropServices;
-using Terminal.Gui.ConsoleDrivers;
 
 namespace Terminal.Gui;