Fixes #2913 - remove Disposed views from Responder.instances when Deb…

…ugging (#2914)

* 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 <john.zuechler@eks-intec.de>
Co-authored-by: BDisp <bd.bdisp@gmail.com>

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;
 		}
 
 		/// <summary>
@@ -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;
-
 		/// <summary>
 		/// Returns a <see cref="Pos"/> object that can be used to center the <see cref="View"/>
 		/// </summary>
@@ -198,9 +193,7 @@ public override string ToString ()
 		/// </example>
 		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;
-
 		/// <summary>
 		/// Adds a <see cref="Terminal.Gui.Pos"/> to a <see cref="Terminal.Gui.Pos"/>, yielding a new <see cref="Pos"/>.
 		/// </summary>
@@ -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;
 		}
 
 		/// <summary>
@@ -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,20 +541,13 @@ internal override int Anchor (int width)
 			public override bool Equals (object other) => other is DimFill fill && fill.margin == margin;
 		}
 
-		static DimFill zeroMargin;
-
 		/// <summary>
 		/// Initializes a new instance of the <see cref="Dim"/> class that fills the dimension, but leaves the specified number of colums for a margin.
 		/// </summary>
 		/// <returns>The Fill dimension.</returns>
 		/// <param name="margin">Margin to use.</param>
 		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;
-
 		/// <summary>
 		/// Adds a <see cref="Terminal.Gui.Dim"/> to a <see cref="Terminal.Gui.Dim"/>, yielding a new <see cref="Dim"/>.
 		/// </summary>
@@ -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;
 		}
 
 		/// <summary>
@@ -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 ();
 			}
 		}
 

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;
 		}
-		
+
 		/// <summary>
 		/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
 		/// </summary>
@@ -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
 		}
 	}

Terminal.Gui/Core/View.cs

@@ -2934,6 +2934,10 @@ protected bool OnMouseClick (MouseEventArgs args)
 		/// <inheritdoc/>
 		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);

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
 		}
 

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);

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
 		}