Forráskód Böngészése

Add fencing to prevent mixing Application models

Co-authored-by: tig <[email protected]>
copilot-swe-agent[bot] 3 hete
szülő
commit
071444ee59

+ 17 - 2
Terminal.Gui/App/Application.Lifecycle.cs

@@ -18,7 +18,15 @@ public static partial class Application // Lifecycle (Init/Shutdown)
     ///     <see cref="IApplication"/> instance for all subsequent application operations.
     /// </remarks>
     /// <returns>A new <see cref="IApplication"/> instance.</returns>
-    public static IApplication Create () { return new ApplicationImpl (); }
+    /// <exception cref="InvalidOperationException">
+    ///     Thrown if the legacy static Application model has already been used in this process.
+    /// </exception>
+    public static IApplication Create ()
+    {
+        ApplicationImpl.MarkInstanceBasedModelUsed ();
+
+        return new ApplicationImpl ();
+    }
 
     /// <inheritdoc cref="IApplication.Init"/>
     [RequiresUnreferencedCode ("AOT")]
@@ -65,5 +73,12 @@ public static partial class Application // Lifecycle (Init/Shutdown)
     // guaranteeing that the state of this singleton is deterministic when Init
     // starts running and after Shutdown returns.
     [Obsolete ("The legacy static Application object is going away.")]
-    internal static void ResetState (bool ignoreDisposed = false) => ApplicationImpl.Instance?.ResetState (ignoreDisposed);
+    internal static void ResetState (bool ignoreDisposed = false)
+    {
+        // Reset the model usage tracking first to allow access to Instance if needed
+        ApplicationImpl.ResetModelUsageTracking ();
+
+        // Now safe to access Instance for cleanup
+        ApplicationImpl.Instance?.ResetState (ignoreDisposed);
+    }
 }

+ 4 - 0
Terminal.Gui/App/ApplicationImpl.Lifecycle.cs

@@ -273,6 +273,10 @@ public partial class ApplicationImpl
         // gui.cs does no longer process any callbacks. See #1084 for more details:
         // (https://github.com/gui-cs/Terminal.Gui/issues/1084).
         SynchronizationContext.SetSynchronizationContext (null);
+
+        // === 12. Reset application model usage tracking ===
+        // Reset the model usage tracking to allow the process to use either model after shutdown
+        ResetModelUsageTracking ();
     }
 
     /// <summary>

+ 68 - 1
Terminal.Gui/App/ApplicationImpl.cs

@@ -21,6 +21,11 @@ public partial class ApplicationImpl : IApplication
 
     #region Singleton
 
+    /// <summary>
+    ///     Tracks which application model has been used in this process.
+    /// </summary>
+    private static ApplicationModelUsage _modelUsage = ApplicationModelUsage.None;
+
     /// <summary>
     ///     Configures the singleton instance of <see cref="Application"/> to use the specified backend implementation.
     /// </summary>
@@ -33,10 +38,72 @@ public partial class ApplicationImpl : IApplication
     /// <summary>
     ///     Gets the currently configured backend implementation of <see cref="Application"/> gateway methods.
     /// </summary>
-    public static IApplication Instance => _instance ??= new ApplicationImpl ();
+    public static IApplication Instance
+    {
+        get
+        {
+            // If an instance already exists, return it without fence checking
+            // This allows for cleanup/reset operations
+            if (_instance is { })
+            {
+                return _instance;
+            }
+
+            // Only check the fence when creating a new instance
+            if (_modelUsage == ApplicationModelUsage.InstanceBased)
+            {
+                throw new InvalidOperationException (
+                    "Cannot use legacy static Application model (Application.Init/ApplicationImpl.Instance) after using modern instance-based model (Application.Create). " +
+                    "Use only one model per process.");
+            }
+
+            _modelUsage = ApplicationModelUsage.LegacyStatic;
+
+            return _instance = new ApplicationImpl ();
+        }
+    }
+
+    /// <summary>
+    ///     INTERNAL: Marks that the instance-based model has been used. Called by Application.Create().
+    /// </summary>
+    internal static void MarkInstanceBasedModelUsed ()
+    {
+        if (_modelUsage == ApplicationModelUsage.LegacyStatic)
+        {
+            throw new InvalidOperationException (
+                "Cannot use modern instance-based model (Application.Create) after using legacy static Application model (Application.Init/ApplicationImpl.Instance). " +
+                "Use only one model per process.");
+        }
+
+        _modelUsage = ApplicationModelUsage.InstanceBased;
+    }
+
+    /// <summary>
+    ///     INTERNAL: Resets the model usage tracking. Only for testing purposes.
+    /// </summary>
+    internal static void ResetModelUsageTracking ()
+    {
+        _modelUsage = ApplicationModelUsage.None;
+        _instance = null;
+    }
 
     #endregion Singleton
 
+    /// <summary>
+    ///     Defines the different application usage models.
+    /// </summary>
+    private enum ApplicationModelUsage
+    {
+        /// <summary>No model has been used yet.</summary>
+        None,
+
+        /// <summary>Legacy static model (Application.Init/ApplicationImpl.Instance).</summary>
+        LegacyStatic,
+
+        /// <summary>Modern instance-based model (Application.Create).</summary>
+        InstanceBased
+    }
+
     private string? _driverName;
 
     #region Input

+ 152 - 0
Tests/UnitTestsParallelizable/Application/ApplicationModelFencingTests.cs

@@ -0,0 +1,152 @@
+namespace UnitTests.ApplicationTests;
+
+/// <summary>
+///     Tests to ensure that mixing legacy static Application and modern instance-based models
+///     throws appropriate exceptions.
+/// </summary>
+[Collection ("Global Test Setup")]
+public class ApplicationModelFencingTests
+{
+    public ApplicationModelFencingTests ()
+    {
+        // Reset the model usage tracking before each test
+        ApplicationImpl.ResetModelUsageTracking ();
+    }
+
+    [Fact]
+    public void Create_ThenInstanceAccess_ThrowsInvalidOperationException ()
+    {
+        // Create a modern instance-based application
+        IApplication app = Application.Create ();
+
+        // Attempting to access the legacy static instance should throw
+        InvalidOperationException ex = Assert.Throws<InvalidOperationException> (() =>
+        {
+            IApplication _ = ApplicationImpl.Instance;
+        });
+
+        Assert.Contains ("Cannot use legacy static Application model", ex.Message);
+        Assert.Contains ("after using modern instance-based model", ex.Message);
+
+        // Clean up
+        app.Shutdown ();
+    }
+
+    [Fact]
+    public void InstanceAccess_ThenCreate_ThrowsInvalidOperationException ()
+    {
+        // Access the legacy static instance
+        IApplication staticInstance = ApplicationImpl.Instance;
+
+        // Attempting to create a modern instance-based application should throw
+        InvalidOperationException ex = Assert.Throws<InvalidOperationException> (() =>
+        {
+            IApplication _ = Application.Create ();
+        });
+
+        Assert.Contains ("Cannot use modern instance-based model", ex.Message);
+        Assert.Contains ("after using legacy static Application model", ex.Message);
+
+        // Clean up
+        staticInstance.Shutdown ();
+    }
+
+    [Fact]
+    public void Init_ThenCreate_ThrowsInvalidOperationException ()
+    {
+        // Initialize using legacy static API
+        IApplication staticInstance = ApplicationImpl.Instance;
+        staticInstance.Init ("fake");
+
+        // Attempting to create a modern instance-based application should throw
+        InvalidOperationException ex = Assert.Throws<InvalidOperationException> (() =>
+        {
+            IApplication _ = Application.Create ();
+        });
+
+        Assert.Contains ("Cannot use modern instance-based model", ex.Message);
+        Assert.Contains ("after using legacy static Application model", ex.Message);
+
+        // Clean up
+        staticInstance.Shutdown ();
+    }
+
+    [Fact]
+    public void Create_ThenInit_ThrowsInvalidOperationException ()
+    {
+        // Create a modern instance-based application
+        IApplication app = Application.Create ();
+        app.Init ("fake");
+
+        // Attempting to access the legacy static instance should throw
+        // (Init calls ApplicationImpl.Instance internally)
+        InvalidOperationException ex = Assert.Throws<InvalidOperationException> (() =>
+        {
+            IApplication _ = ApplicationImpl.Instance;
+        });
+
+        Assert.Contains ("Cannot use legacy static Application model", ex.Message);
+        Assert.Contains ("after using modern instance-based model", ex.Message);
+
+        // Clean up
+        app.Shutdown ();
+    }
+
+    [Fact]
+    public void MultipleCreate_Calls_DoNotThrow ()
+    {
+        // Multiple calls to Create should not throw
+        IApplication app1 = Application.Create ();
+        IApplication app2 = Application.Create ();
+        IApplication app3 = Application.Create ();
+
+        Assert.NotNull (app1);
+        Assert.NotNull (app2);
+        Assert.NotNull (app3);
+
+        // Clean up
+        app1.Shutdown ();
+        app2.Shutdown ();
+        app3.Shutdown ();
+    }
+
+    [Fact]
+    public void MultipleInstanceAccess_DoesNotThrow ()
+    {
+        // Multiple accesses to Instance should not throw (it's a singleton)
+        IApplication instance1 = ApplicationImpl.Instance;
+        IApplication instance2 = ApplicationImpl.Instance;
+        IApplication instance3 = ApplicationImpl.Instance;
+
+        Assert.NotNull (instance1);
+        Assert.Same (instance1, instance2);
+        Assert.Same (instance2, instance3);
+
+        // Clean up
+        instance1.Shutdown ();
+    }
+
+    [Fact]
+    public void ResetModelUsageTracking_AllowsSwitchingModels ()
+    {
+        // Use modern model
+        IApplication app1 = Application.Create ();
+        app1.Shutdown ();
+
+        // Reset the tracking
+        ApplicationImpl.ResetModelUsageTracking ();
+
+        // Should now be able to use legacy model
+        IApplication staticInstance = ApplicationImpl.Instance;
+        Assert.NotNull (staticInstance);
+        staticInstance.Shutdown ();
+
+        // Reset again
+        ApplicationImpl.ResetModelUsageTracking ();
+
+        // Should be able to use modern model again
+        IApplication app2 = Application.Create ();
+        Assert.NotNull (app2);
+        app2.Shutdown ();
+    }
+}