Просмотр исходного кода

Delegate the disposable handling to the caller.

BDisp 1 год назад
Родитель
Сommit
163c0384a5
3 измененных файлов с 103 добавлено и 13 удалено
  1. 46 13
      Terminal.Gui/Application.cs
  2. 1 0
      UICatalog/UICatalog.cs
  3. 56 0
      UnitTests/Dialogs/DialogTests.cs

+ 46 - 13
Terminal.Gui/Application.cs

@@ -88,12 +88,21 @@ public static partial class Application
         foreach (Toplevel t in _topLevels)
         {
             t.Running = false;
-            t.Dispose ();
+#if DEBUG_IDISPOSABLE
+            // Don't dispose the tolevels. It's up to caller dispose them
+            Debug.Assert (t.WasDisposed);
+#endif
         }
 
         _topLevels.Clear ();
         Current = null;
-        Top?.Dispose ();
+#if DEBUG_IDISPOSABLE
+        // Don't dispose the Top. It's up to caller dispose it
+        if (Top is { })
+        {
+            Debug.Assert (Top.WasDisposed);
+        }
+#endif
         Top = null;
 
         // MainLoop stuff
@@ -378,6 +387,10 @@ public static partial class Application
 
 #if DEBUG_IDISPOSABLE
         Debug.Assert (!Toplevel.WasDisposed);
+        if (_latestClosedRunStateToplevel is { } && _latestClosedRunStateToplevel != Toplevel)
+        {
+            Debug.Assert (_latestClosedRunStateToplevel.WasDisposed);
+        }
 #endif
 
         if (Toplevel.IsOverlappedContainer && OverlappedTop != Toplevel && OverlappedTop is { })
@@ -399,18 +412,36 @@ public static partial class Application
 
         lock (_topLevels)
         {
-            // If Top was already initialized with Init, and Begin has never been called
-            // Top was not added to the Toplevels Stack. It will thus never get disposed.
-            // Clean it up here:
             if (Top is { } && Toplevel != Top && !_topLevels.Contains (Top))
             {
-                Top.Dispose ();
-                Top = null;
+#if DEBUG_IDISPOSABLE
+                // This assertion confirm if the Top was already disposed
+                Debug.Assert (Top.WasDisposed);
+                Debug.Assert (Top == _latestClosedRunStateToplevel);
+#endif
+                // If Top was already disposed and isn't on the Toplevels Stack,
+                // clean it up here if is the same as _latestClosedRunStateToplevel
+                if (Top == _latestClosedRunStateToplevel)
+                {
+                    Top = null;
+                }
+                else
+                {
+                    // Probably this will never hit
+                    throw new ObjectDisposedException (Top.GetType ().FullName);
+                }
             }
-            else if (Top is { } && Toplevel != Top && _topLevels.Contains (Top))
+            else if (OverlappedTop is { } && Toplevel != Top && _topLevels.Contains (Top))
             {
                 Top.OnLeave (Toplevel);
             }
+            else if (OverlappedTop is null && Top is { } && Toplevel != Top && _topLevels.Contains (Top))
+            {
+#if DEBUG_IDISPOSABLE
+                // Probably this will never hit
+                Debug.Assert (Top.WasDisposed);
+#endif
+            }
 
             // BUGBUG: We should not depend on `Id` internally. 
             // BUGBUG: It is super unclear what this code does anyway.
@@ -452,7 +483,7 @@ public static partial class Application
 
         var refreshDriver = true;
 
-        if (OverlappedTop == null
+        if (OverlappedTop is null
             || Toplevel.IsOverlappedContainer
             || (Current?.Modal == false && Toplevel.Modal)
             || (Current?.Modal == false && !Toplevel.Modal)
@@ -947,6 +978,8 @@ public static partial class Application
         }
     }
 
+    private static Toplevel _latestClosedRunStateToplevel;
+
     /// <summary>
     ///     Building block API: completes the execution of a <see cref="Toplevel"/> that was started with
     ///     <see cref="Begin(Toplevel)"/> .
@@ -1015,10 +1048,10 @@ public static partial class Application
             Refresh ();
         }
 
-        // Always dispose runState.Toplevel here. If it is not the same as
-        // the current in the RunIteration, it will be fixed later in the
-        // next RunIteration.
-        runState.Toplevel?.Dispose ();
+        // Don't dispose runState.Toplevel. It's up to caller dispose it
+        // If it's not the same as the current in the RunIteration,
+        // it will be fixed later in the next RunIteration.
+        _latestClosedRunStateToplevel = runState.Toplevel;
         runState.Toplevel = null;
         runState.Dispose ();
     }

+ 1 - 0
UICatalog/UICatalog.cs

@@ -1070,6 +1070,7 @@ internal class UICatalogApp
         {
             Applied -= ConfigAppliedHandler;
             Unloaded -= UnloadedHandler;
+            Dispose ();
         }
     }
 

+ 56 - 0
UnitTests/Dialogs/DialogTests.cs

@@ -1365,4 +1365,60 @@ public class DialogTests
             RequestStop ();
         }
     }
+
+    [Fact]
+    [AutoInitShutdown]
+    public void Can_Access_Cancel_Property_After_Run ()
+    {
+        Dialog dlg = new ();
+
+        dlg.Ready += Dlg_Ready;
+
+        Run (dlg);
+
+#if DEBUG_IDISPOSABLE
+        Assert.False (dlg.WasDisposed);
+        Assert.False (Top.WasDisposed);
+        Assert.Equal (dlg, Top);
+#endif
+
+        Assert.True (dlg.Canceled);
+
+        // Run it again is possible because it isn't disposed yet
+        Run (dlg);
+
+        // Run another view without dispose the prior will throw an assertion
+#if DEBUG_IDISPOSABLE
+        Dialog dlg2 = new ();
+        dlg2.Ready += Dlg_Ready;
+        var exception = Record.Exception (() => Run (dlg2));
+        Assert.NotNull (exception);
+
+        dlg.Dispose();
+        // Now it's possible to tun dlg2 without throw
+        Run (dlg2);
+
+        Assert.True (dlg.WasDisposed);
+        Assert.False (Top.WasDisposed);
+        Assert.Equal (dlg2, Top);
+        Assert.False (dlg2.WasDisposed);
+
+        dlg2.Dispose ();
+        // Now an assertion will throw accessing the Canceled property
+        exception = Record.Exception (() => Assert.True (dlg.Canceled));
+        Assert.NotNull (exception);
+        Assert.True (Top.WasDisposed);
+        Shutdown ();
+        Assert.True (dlg2.WasDisposed);
+        Assert.Null (Top);
+#endif
+
+        return;
+
+        void Dlg_Ready (object sender, EventArgs e)
+        {
+            ((Dialog)sender).Canceled = true;
+            RequestStop ();
+        }
+    }
 }