From a8d2f8d7335f12f0a7fe7121d0fd7811777e7126 Mon Sep 17 00:00:00 2001 From: usr <81042605+a-usr@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:02:46 +0200 Subject: [PATCH] Fixes #2913 - remove Disposed views from Responder.instances when Debugging (#2914) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix issue #2913 * remove disposed instances from responder.instances (optional) * Ensures only clear Instances if they really was disposed. * Fix unit tests. * Update RunStateTests.cs Minor format change to trigger new Unittest execution * Fixes Pos/Dim static fields not being disposing. --------- Co-authored-by: John Züchler Co-authored-by: BDisp --- Terminal.Gui/Core/PosDim.cs | 56 ++++++----------------- Terminal.Gui/Core/Responder.cs | 8 +++- Terminal.Gui/Core/View.cs | 4 ++ UnitTests/Application/ApplicationTests.cs | 3 +- UnitTests/Application/RunStateTests.cs | 1 + UnitTests/TopLevels/MdiTests.cs | 8 +--- 6 files changed, 29 insertions(+), 51 deletions(-) diff --git a/Terminal.Gui/Core/PosDim.cs b/Terminal.Gui/Core/PosDim.cs index 1169fa0493..7a5f23b785 100644 --- a/Terminal.Gui/Core/PosDim.cs +++ b/Terminal.Gui/Core/PosDim.cs @@ -118,8 +118,6 @@ public static Pos Percent (float n) return new PosFactor (n / 100); } - static PosAnchorEnd endNoMargin; - internal class PosAnchorEnd : Pos { int n; @@ -137,6 +135,10 @@ public override string ToString () { return $"Pos.AnchorEnd(margin={n})"; } + + public override int GetHashCode () => n.GetHashCode (); + + public override bool Equals (object other) => other is PosAnchorEnd anchorEnd && anchorEnd.n == n; } /// @@ -158,11 +160,6 @@ public static Pos AnchorEnd (int margin = 0) if (margin < 0) throw new ArgumentException ("Margin must be positive"); - if (margin == 0) { - if (endNoMargin == null) - endNoMargin = new PosAnchorEnd (0); - return endNoMargin; - } return new PosAnchorEnd (margin); } @@ -178,8 +175,6 @@ public override string ToString () } } - static PosCenter pCenter; - /// /// Returns a object that can be used to center the /// @@ -198,9 +193,7 @@ public override string ToString () /// public static Pos Center () { - if (pCenter == null) - pCenter = new PosCenter (); - return pCenter; + return new PosCenter (); } internal class PosAbsolute : Pos { @@ -269,8 +262,6 @@ public override string ToString () } - static PosCombine posCombine; - /// /// Adds a to a , yielding a new . /// @@ -280,12 +271,11 @@ public override string ToString () public static Pos operator + (Pos left, Pos right) { if (left is PosAbsolute && right is PosAbsolute) { - posCombine = null; return new PosAbsolute (left.Anchor (0) + right.Anchor (0)); } PosCombine newPos = new PosCombine (true, left, right); SetPosCombine (left, newPos); - return posCombine = newPos; + return newPos; } /// @@ -297,21 +287,18 @@ public override string ToString () public static Pos operator - (Pos left, Pos right) { if (left is PosAbsolute && right is PosAbsolute) { - posCombine = null; return new PosAbsolute (left.Anchor (0) - right.Anchor (0)); } PosCombine newPos = new PosCombine (false, left, right); SetPosCombine (left, newPos); - return posCombine = newPos; + return newPos; } static void SetPosCombine (Pos left, PosCombine newPos) { - if (posCombine?.ToString () != newPos.ToString ()) { - var view = left as PosView; - if (view != null) { - view.Target.SetNeedsLayout (); - } + var view = left as PosView; + if (view != null) { + view.Target.SetNeedsLayout (); } } @@ -554,8 +541,6 @@ internal override int Anchor (int width) public override bool Equals (object other) => other is DimFill fill && fill.margin == margin; } - static DimFill zeroMargin; - /// /// Initializes a new instance of the class that fills the dimension, but leaves the specified number of colums for a margin. /// @@ -563,11 +548,6 @@ internal override int Anchor (int width) /// Margin to use. public static Dim Fill (int margin = 0) { - if (margin == 0) { - if (zeroMargin == null) - zeroMargin = new DimFill (0); - return zeroMargin; - } return new DimFill (margin); } @@ -618,8 +598,6 @@ public override string ToString () } - static DimCombine dimCombine; - /// /// Adds a to a , yielding a new . /// @@ -629,12 +607,11 @@ public override string ToString () public static Dim operator + (Dim left, Dim right) { if (left is DimAbsolute && right is DimAbsolute) { - dimCombine = null; return new DimAbsolute (left.Anchor (0) + right.Anchor (0)); } DimCombine newDim = new DimCombine (true, left, right); SetDimCombine (left, newDim); - return dimCombine = newDim; + return newDim; } /// @@ -646,21 +623,18 @@ public override string ToString () public static Dim operator - (Dim left, Dim right) { if (left is DimAbsolute && right is DimAbsolute) { - dimCombine = null; return new DimAbsolute (left.Anchor (0) - right.Anchor (0)); } DimCombine newDim = new DimCombine (false, left, right); SetDimCombine (left, newDim); - return dimCombine = newDim; + return newDim; } static void SetDimCombine (Dim left, DimCombine newPos) { - if (dimCombine?.ToString () != newPos.ToString ()) { - var view = left as DimView; - if (view != null) { - view.Target.SetNeedsLayout (); - } + var view = left as DimView; + if (view != null) { + view.Target.SetNeedsLayout (); } } diff --git a/Terminal.Gui/Core/Responder.cs b/Terminal.Gui/Core/Responder.cs index 7b92f8a04d..d586b5d386 100644 --- a/Terminal.Gui/Core/Responder.cs +++ b/Terminal.Gui/Core/Responder.cs @@ -15,7 +15,7 @@ using System; using System.Collections.Generic; -using System.Diagnostics; +using System.Linq; using System.Reflection; namespace Terminal.Gui { @@ -255,7 +255,7 @@ internal static bool IsOverridden (Responder subclass, string method) } return m.GetBaseDefinition ().DeclaringType != m.DeclaringType; } - + /// /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. /// @@ -291,6 +291,10 @@ public void Dispose () GC.SuppressFinalize (this); #if DEBUG_IDISPOSABLE WasDisposed = true; + + foreach (var instance in Instances.Where (x => x.WasDisposed).ToList ()) { + Instances.Remove (instance); + } #endif } } diff --git a/Terminal.Gui/Core/View.cs b/Terminal.Gui/Core/View.cs index b2805320b1..2d34647a08 100644 --- a/Terminal.Gui/Core/View.cs +++ b/Terminal.Gui/Core/View.cs @@ -2934,6 +2934,10 @@ protected bool OnMouseClick (MouseEventArgs args) /// protected override void Dispose (bool disposing) { + height = null; + width = null; + x = null; + y = null; for (var i = InternalSubviews.Count - 1; i >= 0; i--) { var subview = InternalSubviews [i]; Remove (subview); diff --git a/UnitTests/Application/ApplicationTests.cs b/UnitTests/Application/ApplicationTests.cs index 9a293a1b50..d36a1c8cd0 100644 --- a/UnitTests/Application/ApplicationTests.cs +++ b/UnitTests/Application/ApplicationTests.cs @@ -91,8 +91,7 @@ public void Init_Shutdown_Toplevel_Not_Disposed () Application.Shutdown (); #if DEBUG_IDISPOSABLE - Assert.Single (Responder.Instances); - Assert.True (Responder.Instances [0].WasDisposed); + Assert.Empty (Responder.Instances); #endif } diff --git a/UnitTests/Application/RunStateTests.cs b/UnitTests/Application/RunStateTests.cs index ffae6abba5..500af598a8 100644 --- a/UnitTests/Application/RunStateTests.cs +++ b/UnitTests/Application/RunStateTests.cs @@ -63,6 +63,7 @@ public void Dispose_Cleans_Up_RunState () void Init () { Application.Init (new FakeDriver ()); + Assert.NotNull (Application.Driver); Assert.NotNull (Application.MainLoop); Assert.NotNull (SynchronizationContext.Current); diff --git a/UnitTests/TopLevels/MdiTests.cs b/UnitTests/TopLevels/MdiTests.cs index 9da88bacec..2564def241 100644 --- a/UnitTests/TopLevels/MdiTests.cs +++ b/UnitTests/TopLevels/MdiTests.cs @@ -32,9 +32,7 @@ public void Dispose_Toplevel_IsMdiContainer_False_With_Begin_End () Application.Shutdown (); #if DEBUG_IDISPOSABLE - Assert.Equal (2, Responder.Instances.Count); - Assert.True (Responder.Instances [0].WasDisposed); - Assert.True (Responder.Instances [1].WasDisposed); + Assert.Empty (Responder.Instances); #endif } @@ -49,9 +47,7 @@ public void Dispose_Toplevel_IsMdiContainer_True_With_Begin () Application.Shutdown (); #if DEBUG_IDISPOSABLE - Assert.Equal (2, Responder.Instances.Count); - Assert.True (Responder.Instances [0].WasDisposed); - Assert.True (Responder.Instances [1].WasDisposed); + Assert.Empty (Responder.Instances); #endif }