[UIKit] UIGestureRecognizer, support a way of unsubscribing without c…

…reating cycles (#4729)

* [UIKit] UIGestureRecognizer, support a way of unsubscribing without creating cycles

This now tracks all the uses of AddTarget with delegates by recording
the Token/Selector pair and making `Dispose()` release all the linkage
as well as providing an enumerator that can be used to get all the
registered Action handlers - this can then be used with .First() and
then passed to `RemoveTarget`.

This addresses #4190

This initial patch is here for discussion of the approach, want to
review and test this before we merge.

* Simplify code a little bit.

* Add test.

* [tests] Add an NSAutoreleasePool to make GestureRecognizerTest.NoStrongCycles happy on 32-bit.

src/UIKit/UIGestureRecognizer.cs

@@ -12,13 +12,17 @@
 
 using System;
 using System.Collections;
+using System.Collections.Generic;
 using Foundation; 
 using ObjCRuntime;
 using CoreGraphics;
 
 namespace UIKit {
 	public partial class UIGestureRecognizer {
-		object recognizers;
+		//
+		// Tracks the targets (NSObject, which we always enforce to be Token) to the Selector the point to, used when disposing
+		//
+		Dictionary<Token,IntPtr> recognizers = new Dictionary<Token,IntPtr> ();
 		const string tsel = "target";
 		internal const string parametrized_selector = "target:";
 #if !XAMCORE_2_0
@@ -30,18 +34,26 @@ public partial class UIGestureRecognizer {
 		{
 		}
 
+		// Called by the Dispose() method
+		void OnDispose ()
+		{
+			foreach (var kv in recognizers)
+				RemoveTarget (kv.Key, kv.Value);
+			recognizers = null;
+		}
+
 		//
 		// Signature swapped, this is only used so we can store the "token" in recognizers
 		//
 		public UIGestureRecognizer (Selector sel, Token token) : this (token, sel)
 		{
-			recognizers = token;
+			recognizers [token] = sel.Handle;
 			MarkDirty ();
 		}
 
 		internal UIGestureRecognizer (IntPtr sel, Token token) : this (token, sel)
 		{
-			recognizers = token;
+			recognizers [token] = sel;
 			MarkDirty ();
 		}
 
@@ -111,17 +123,7 @@ void RegisterTarget (Token target, IntPtr sel)
 		{
 			AddTarget (target, sel);
 			MarkDirty ();
-			if (recognizers == null)
-				recognizers = target;
-			else {
-				Hashtable table = recognizers as Hashtable;
-				if (table == null){
-					table = new Hashtable ();
-					table [recognizers] = recognizers;
-					recognizers = table;
-				}
-				table [target] = target;
-			}
+			recognizers [target] = sel;
 		}
 
 		public void RemoveTarget (Token token)
@@ -130,12 +132,16 @@ public void RemoveTarget (Token token)
 				throw new ArgumentNullException ("token");
 			if (recognizers == null)
 				return;
-			if (recognizers == token)
-				recognizers = null;
-			Hashtable asHash = recognizers as Hashtable;
-			if (asHash != null)
-				asHash.Remove (token);
-			RemoveTarget (token, token is ParametrizedDispatch ? Selector.GetHandle (parametrized_selector) : Selector.GetHandle (tsel));
+			if (recognizers.Remove (token, out var sel))
+				RemoveTarget (token, sel);
+		}
+
+		//
+		// Used to enumerate all the registered handlers for this UIGestureRecognizer
+		//
+		public IEnumerable<Token> GetTargets ()
+		{
+			return (IEnumerable<Token>) recognizers?.Keys ?? Array.Empty<Token> ();
 		}
 	}
 

src/uikit.cs

@@ -5546,6 +5546,7 @@ partial interface UIFontDescriptor : NSSecureCoding, NSCopying {
 
 #if !WATCH
 	[BaseType (typeof(NSObject), Delegates=new string [] {"WeakDelegate"}, Events=new Type[] {typeof (UIGestureRecognizerDelegate)})]
+	[Dispose ("OnDispose ();")]
 	interface UIGestureRecognizer {
 		[DesignatedInitializer]
 		[Export ("initWithTarget:action:")]

tests/monotouch-test/UIKit/GestureRecognizerTest.cs

@@ -10,6 +10,7 @@
 #if !__WATCHOS__ && !MONOMAC
 
 using System;
+using System.Collections.Generic;
 #if XAMCORE_2_0
 using Foundation;
 using UIKit;
@@ -37,6 +38,66 @@ public void Null ()
 				gr.RemoveTarget (null, null);
 			}
 		}
+
+		[Test]
+		public void NoStrongCycles ()
+		{
+			bool finalizedAnyCtor = false;
+			bool finalizedAnyAddTarget1 = false;
+			bool finalizedAnyAddTarget2 = false;
+
+			// Add the gesture recognizers to a list so that they're not collected until after the test
+			// This is to avoid false positives (the callback should be collectible already after disposing the gesture recognizer).
+			var list = new List<UIGestureRecognizer> ();
+
+			var pool = new NSAutoreleasePool ();
+			for (var k = 0; k < 10; k++) {
+				{
+					var notifier = new FinalizerNotifier (() => finalizedAnyCtor = true);
+					using (var gr = new UIGestureRecognizer (() => {
+						GC.KeepAlive (notifier); // Make sure the 'notifier' instance is only collected if the delegate to UIGestureRecognizer is collectable.
+					})) {
+						list.Add (gr);
+					}
+				}
+				{
+					var notifier = new FinalizerNotifier (() => finalizedAnyAddTarget1 = true);
+					using (var gr = new UIGestureRecognizer ()) {
+						gr.AddTarget (() => { GC.KeepAlive (notifier); });
+						list.Add (gr);
+					}
+				}
+				{
+					var notifier = new FinalizerNotifier (() => finalizedAnyAddTarget2 = true);
+					using (var gr = new UIGestureRecognizer ()) {
+						gr.AddTarget ((obj) => { GC.KeepAlive (notifier); });
+						list.Add (gr);
+					}
+				}
+			}
+			pool.Dispose ();
+
+			TestRuntime.RunAsync (DateTime.Now.AddSeconds (1), () => { GC.Collect (); }, () => finalizedAnyCtor && finalizedAnyAddTarget1 && finalizedAnyAddTarget2);
+			Assert.IsTrue (finalizedAnyCtor, "Any finalized");
+			Assert.IsTrue (finalizedAnyAddTarget1, "AddTarget1 finalized");
+			Assert.IsTrue (finalizedAnyAddTarget2, "AddTarget2 finalized");
+
+			GC.KeepAlive (list);
+		}
+
+		class FinalizerNotifier
+		{
+			public Action Action;
+			public FinalizerNotifier (Action action)
+			{
+				Action = action;
+			}
+			~FinalizerNotifier ()
+			{
+				if (Action != null)
+					Action ();
+			}
+		}
 	}
 }