Fixes #3675. Adds ObjectDisposedExceptions

Terminal.Gui/Application/Application.Keyboard.cs

@@ -147,6 +147,9 @@ public static bool OnKeyDown (Key keyEvent)
         {
             foreach (Toplevel topLevel in TopLevels.ToList ())
             {
+#if DEBUG_IDISPOSABLE
+                ObjectDisposedException.ThrowIf (topLevel is { WasDisposed: true }, typeof (View));
+#endif
                 if (topLevel.NewKeyDownEvent (keyEvent))
                 {
                     return true;
@@ -160,6 +163,9 @@ public static bool OnKeyDown (Key keyEvent)
         }
         else
         {
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (Current is { WasDisposed: true }, typeof (View));
+#endif
             if (Current.NewKeyDownEvent (keyEvent))
             {
                 return true;

Terminal.Gui/Application/Application.Mouse.cs

@@ -74,6 +74,9 @@ private static bool OnGrabbingMouse (View? view)
             return false;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (view.WasDisposed, typeof (View));
+#endif
         var evArgs = new GrabMouseEventArgs (view);
         GrabbingMouse?.Invoke (view, evArgs);
 
@@ -88,6 +91,10 @@ private static bool OnUnGrabbingMouse (View? view)
             return false;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (view.WasDisposed, typeof (View));
+#endif
+
         var evArgs = new GrabMouseEventArgs (view);
         UnGrabbingMouse?.Invoke (view, evArgs);
 
@@ -102,6 +109,10 @@ private static void OnGrabbedMouse (View? view)
             return;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (view.WasDisposed, typeof (View));
+#endif
+
         GrabbedMouse?.Invoke (view, new (view));
     }
 
@@ -113,6 +124,10 @@ private static void OnUnGrabbedMouse (View? view)
             return;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (view.WasDisposed, typeof (View));
+#endif
+
         UnGrabbedMouse?.Invoke (view, new (view));
     }
 
@@ -139,10 +154,21 @@ internal static void OnMouseEvent (MouseEvent mouseEvent)
             return;
         }
 
+#if DEBUG_IDISPOSABLE
+        if (Current is { })
+        {
+            ObjectDisposedException.ThrowIf (Current.WasDisposed, typeof (View));
+        }
+#endif
+
         var view = View.FindDeepestView (Current, mouseEvent.Position);
 
         if (view is { })
         {
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (view.WasDisposed, typeof (View));
+#endif
+
             mouseEvent.View = view;
         }
 
@@ -250,6 +276,10 @@ internal static void OnMouseEvent (MouseEvent mouseEvent)
         }
         else if (MouseEnteredView != view)
         {
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (MouseEnteredView is { WasDisposed: true }, typeof (View));
+#endif
+
             MouseEnteredView.NewMouseLeaveEvent (me);
             view.NewMouseEnterEvent (me);
             MouseEnteredView = view;
@@ -262,6 +292,10 @@ internal static void OnMouseEvent (MouseEvent mouseEvent)
 
         WantContinuousButtonPressedView = view.WantContinuousButtonPressed ? view : null;
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (WantContinuousButtonPressedView is { WasDisposed: true }, typeof (View));
+#endif
+
         //Debug.WriteLine ($"OnMouseEvent: ({a.MouseEvent.X},{a.MouseEvent.Y}) - {a.MouseEvent.Flags}");
 
         while (view.NewMouseEvent (me) is not true && MouseGrabView is not { })

Terminal.Gui/Application/Application.Run.cs

@@ -436,8 +436,6 @@
 #if DEBUG_IDISPOSABLE
                 Debug.Assert (TopLevels.Count == 0);
 #endif
-                runState.Dispose ();
-
                 return;
             }
 
@@ -571,15 +569,17 @@
     {
         if (MainLoop!.Running && MainLoop.EventsPending ())
         {
-            // Notify Toplevel it's ready
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (state.Toplevel.WasDisposed, typeof (View));
+#endif            // Notify Toplevel it's ready
             if (firstIteration)
             {
                 state.Toplevel.OnReady ();
             }
 
             MainLoop.RunIteration ();
             Iteration?.Invoke (null, new ());
             EnsureModalOrVisibleAlwaysOnTop (state.Toplevel);
 
             // TODO: Overlapped - Move elsewhere
             if (state.Toplevel != Current)
@@ -592,6 +592,10 @@
             }
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (state.Toplevel is { WasDisposed: true }, typeof (View));
+#endif
+
         firstIteration = false;
 
         if (Current == null)
@@ -671,6 +675,10 @@
             top = Current;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (top is { WasDisposed: true }, typeof (View));
+#endif
+
         if (ApplicationOverlapped.OverlappedTop != null
             && top!.IsOverlappedContainer
             && top?.Running == true
@@ -791,13 +799,17 @@
     {
         ArgumentNullException.ThrowIfNull (runState);
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (runState.Toplevel is { WasDisposed: true }, typeof (View));
+#endif
+
         if (ApplicationOverlapped.OverlappedTop is { })
         {
             ApplicationOverlapped.OverlappedTop.OnChildUnloaded (runState.Toplevel);
         }
         else
         {
             runState.Toplevel.OnUnloaded ();
         }
 
         // End the RunState.Toplevel
@@ -878,6 +890,5 @@
         }
 
         runState.Toplevel = null;
-        runState.Dispose ();
     }
 }

Terminal.Gui/Application/ApplicationNavigation.cs

@@ -97,8 +97,14 @@ public static bool IsInHierarchy (View start, View? view)
             return null;
         }
 
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (view is { WasDisposed: true }, typeof (View));
+#endif
         foreach (View v in view.Subviews)
         {
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (v is { WasDisposed: true }, typeof (View));
+#endif
             if (v.HasFocus)
             {
                 return GetDeepestFocusedSubview (v);

Terminal.Gui/Application/RunState.cs

@@ -1,57 +1,12 @@
-namespace Terminal.Gui;
+#nullable enable
+
+namespace Terminal.Gui;
+
+using System.Numerics;
 
 /// <summary>The execution state for a <see cref="Toplevel"/> view.</summary>
-public class RunState : IDisposable
+public sealed record RunState (Toplevel Toplevel) : IEqualityOperators<RunState, RunState, bool>
 {
-    /// <summary>Initializes a new <see cref="RunState"/> class.</summary>
-    /// <param name="view"></param>
-    public RunState (Toplevel view) { Toplevel = view; }
-
     /// <summary>The <see cref="Toplevel"/> belonging to this <see cref="RunState"/>.</summary>
-    public Toplevel Toplevel { get; internal set; }
-
-    /// <summary>Releases all resource used by the <see cref="RunState"/> object.</summary>
-    /// <remarks>Call <see cref="Dispose()"/> when you are finished using the <see cref="RunState"/>.</remarks>
-    /// <remarks>
-    ///     <see cref="Dispose()"/> method leaves the <see cref="RunState"/> in an unusable state. After calling
-    ///     <see cref="Dispose()"/>, you must release all references to the <see cref="RunState"/> so the garbage collector can
-    ///     reclaim the memory that the <see cref="RunState"/> was occupying.
-    /// </remarks>
-    public void Dispose ()
-    {
-        Dispose (true);
-        GC.SuppressFinalize (this);
-#if DEBUG_IDISPOSABLE
-        WasDisposed = true;
-#endif
-    }
-
-    /// <summary>Releases all resource used by the <see cref="RunState"/> object.</summary>
-    /// <param name="disposing">If set to <see langword="true"/> we are disposing and should dispose held objects.</param>
-    protected virtual void Dispose (bool disposing)
-    {
-        if (Toplevel is { } && disposing)
-        {
-            // Previously we were requiring Toplevel be disposed here.
-            // But that is not correct becaue `Begin` didn't create the TopLevel, `Init` did; thus
-            // disposing should be done by `Shutdown`, not `End`.
-            throw new InvalidOperationException (
-                                                 "Toplevel must be null before calling Application.RunState.Dispose"
-                                                );
-        }
-    }
-
-#if DEBUG_IDISPOSABLE
-    /// <summary>For debug (see DEBUG_IDISPOSABLE define) purposes to verify objects are being disposed properly</summary>
-    public bool WasDisposed;
-
-    /// <summary>For debug (see DEBUG_IDISPOSABLE define) purposes to verify objects are being disposed properly</summary>
-    public int DisposedCount = 0;
-
-    /// <summary>For debug (see DEBUG_IDISPOSABLE define) purposes; the runstate instances that have been created</summary>
-    public static List<RunState> Instances = new ();
-
-    /// <summary>Creates a new RunState object.</summary>
-    public RunState () { Instances.Add (this); }
-#endif
+    public Toplevel? Toplevel { get; internal set; } = Toplevel;
 }

Terminal.Gui/View/Adornment/Adornment.cs

@@ -75,6 +75,10 @@ public Thickness Thickness
     /// <summary>Called whenever the <see cref="Thickness"/> property changes.</summary>
     public void OnThicknessChanged ()
     {
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (WasDisposed, typeof (Adornment));
+#endif
+
         ThicknessChanged?.Invoke (this, EventArgs.Empty);
     }
 
@@ -147,6 +151,10 @@ public override Point ScreenToFrame (in Point location)
     /// <summary>Redraws the Adornments that comprise the <see cref="Adornment"/>.</summary>
     public override void OnDrawContent (Rectangle viewport)
     {
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (WasDisposed, typeof (Adornment));
+#endif
+
         if (Thickness == Thickness.Empty)
         {
             return;

Terminal.Gui/View/View.Layout.cs

@@ -60,6 +60,10 @@ public partial class View // Layout APIs
 
             View? subview = null;
 
+#if DEBUG_IDISPOSABLE
+            ObjectDisposedException.ThrowIf (start.WasDisposed, typeof (View));
+#endif
+
             for (int i = start.InternalSubviews.Count - 1; i >= 0; i--)
             {
                 if (start.InternalSubviews [i].Visible

Terminal.Gui/View/View.cs

@@ -224,6 +224,10 @@ public View ()
     /// </remarks>
     public virtual void BeginInit ()
     {
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (WasDisposed, typeof (View));
+#endif
+
         if (IsInitialized)
         {
             throw new InvalidOperationException ("The view is already initialized.");
@@ -432,22 +436,10 @@ public string Title
     {
         get
         {
-#if DEBUG_IDISPOSABLE
-            if (WasDisposed)
-            {
-                throw new ObjectDisposedException (GetType ().FullName);
-            }
-#endif
             return _title;
         }
         set
         {
-#if DEBUG_IDISPOSABLE
-            if (WasDisposed)
-            {
-                throw new ObjectDisposedException (GetType ().FullName);
-            }
-#endif
             if (value == _title)
             {
                 return;
@@ -486,6 +478,10 @@ private void SetTitleTextFormatterSize ()
     /// <summary>Called when the <see cref="View.Title"/> has been changed. Invokes the <see cref="TitleChanged"/> event.</summary>
     protected void OnTitleChanged ()
     {
+#if DEBUG_IDISPOSABLE
+        ObjectDisposedException.ThrowIf (WasDisposed, typeof (View));
+#endif
+
         TitleChanged?.Invoke (this, new (in _title));
     }
 

UICatalog/UICatalog.cs

@@ -364,16 +364,6 @@ private static void VerifyObjectsWereDisposed ()
         }
 
         Responder.Instances.Clear ();
-
-        // Validate there are no outstanding Application.RunState-based instances 
-        // after a scenario was selected to run. This proves the main UI Catalog
-        // 'app' closed cleanly.
-        foreach (RunState? inst in RunState.Instances)
-        {
-            Debug.Assert (inst.WasDisposed);
-        }
-
-        RunState.Instances.Clear ();
 #endif
     }
 

UnitTests/Application/ApplicationTests.cs

@@ -13,7 +13,6 @@
 
 #if DEBUG_IDISPOSABLE
         Responder.Instances.Clear ();
-        RunState.Instances.Clear ();
 #endif
     }
 
@@ -177,7 +176,6 @@
         Application.End (rs);
 
 #if DEBUG_IDISPOSABLE
-        Assert.True (rs.WasDisposed);
         Assert.False (Application.Top.WasDisposed); // Is true because the rs.Toplevel is the same as Application.Top
 #endif
 
@@ -1069,8 +1067,6 @@
                                                              w)); // Invalid - w has been disposed. Run it in debug mode will throw, otherwise the user may want to run it again
         Assert.NotNull (exception);
 
-        exception = Record.Exception (() => Assert.Equal (string.Empty, w.Title)); // Invalid - w has been disposed and cannot be accessed
-        Assert.NotNull (exception);
         exception = Record.Exception (() => w.Title = "NewTitle"); // Invalid - w has been disposed and cannot be accessed
         Assert.NotNull (exception);
 #endif
@@ -1190,7 +1186,7 @@
     #region ShutdownTests
 
     [Fact]
     public async void Shutdown_Allows_Async ()
     {
         var isCompletedSuccessfully = false;
 

UnitTests/Application/KeyboardTests.cs

@@ -15,7 +15,6 @@ public KeyboardTests (ITestOutputHelper output)
         _output = output;
 #if DEBUG_IDISPOSABLE
         Responder.Instances.Clear ();
-        RunState.Instances.Clear ();
 #endif
     }
 

UnitTests/Application/MouseTests.cs

@@ -13,7 +13,6 @@ public MouseTests (ITestOutputHelper output)
         _output = output;
 #if DEBUG_IDISPOSABLE
         Responder.Instances.Clear ();
-        RunState.Instances.Clear ();
 #endif
     }