refactor: Remove reflection usage in Brush

src/Uno.UI.RuntimeTests/Tests/Uno_UI_Helpers/Given_WeakEventManager.cs

@@ -0,0 +1,161 @@
+#nullable enable
+#if HAS_UNO
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Uno.UI.Helpers;
+
+namespace Uno.UI.RuntimeTests.Tests.Uno_UI_Helpers;
+
+[TestClass]
+public class Given_WeakEventManager
+{
+	private sealed class EventPublisher
+	{
+		private readonly WeakEventManager _manager = new();
+
+		internal event Action Event
+		{
+			add => _manager.AddEventHandler(value);
+			remove => _manager.RemoveEventHandler(value);
+		}
+
+		public void RaiseEvent() => _manager.HandleEvent("Event");
+	}
+
+	private sealed class EventSubscriber
+	{
+		public void M() { }
+	}
+
+	[TestMethod]
+	public void When_ManySubscriptions_Then_DoesNotLeak()
+	{
+		var publisher = new EventPublisher();
+		var weakRefs = new List<WeakReference<EventSubscriber>>();
+		Subscribe(publisher, weakRefs);
+
+		for (var i = 0; i < 10; i++)
+		{
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+		}
+
+		foreach (var x in weakRefs)
+		{
+			Assert.IsFalse(x.TryGetTarget(out _));
+		}
+	}
+
+	[TestMethod]
+	public void When_ReEnter_Then_AllHandlersInvokedProperly()
+	{
+		var sut = new WeakEventManager();
+		int handler1Count = 0, handler2Count = 0;
+
+		sut.AddEventHandler(Handler1, "Event1");
+		sut.AddEventHandler(Handler2, "Event1");
+		sut.AddEventHandler(Handler1_2, "Event2");
+		sut.AddEventHandler(Handler2_2, "Event2");
+
+		sut.HandleEvent("Event1");
+
+		Assert.AreEqual(3, handler1Count);
+		Assert.AreEqual(3, handler2Count);
+
+		sut.HandleEvent("Event2");
+
+		Assert.AreEqual(4, handler1Count);
+		Assert.AreEqual(4, handler2Count);
+
+		void Handler1()
+		{
+			handler1Count++;
+			sut.HandleEvent("Event2");
+		}
+
+		void Handler1_2()
+		{
+			handler1Count++;
+		}
+
+		void Handler2()
+		{
+			handler2Count++;
+			sut.HandleEvent("Event2");
+		}
+
+		void Handler2_2()
+		{
+			handler2Count++;
+		}
+	}
+
+	[TestMethod]
+	public void When_UnSubscribeInHandler()
+	{
+		var sut = new WeakEventManager();
+		int handler1Count = 0, handler2Count = 0;
+
+		sut.AddEventHandler(Handler1, "Event1");
+		sut.AddEventHandler(Handler2, "Event1");
+
+		sut.HandleEvent("Event1");
+		sut.HandleEvent("Event1");
+
+		Assert.AreEqual(1, handler1Count);
+		Assert.AreEqual(2, handler2Count);
+
+		void Handler1()
+		{
+			handler1Count++;
+			sut.RemoveEventHandler(Handler1, "Event1");
+		}
+
+		void Handler2()
+		{
+			handler2Count++;
+		}
+	}
+
+	[TestMethod]
+	public void When_UnSubscribeInHandler2()
+	{
+		var pub = new EventPublisher();
+		int handler1Count = 0, handler2Count = 0;
+
+		pub.Event += Handler1;
+		pub.Event += Handler2;
+
+		pub.RaiseEvent();
+		pub.RaiseEvent();
+
+		Assert.AreEqual(1, handler1Count);
+		Assert.AreEqual(2, handler2Count);
+
+		void Handler1()
+		{
+			handler1Count++;
+			pub.Event -= Handler1;
+		}
+
+		void Handler2()
+		{
+			handler2Count++;
+		}
+	}
+
+	private void Subscribe(EventPublisher publisher, List<WeakReference<EventSubscriber>> weakRefs)
+	{
+		for (var i = 0; i < 1000; i++)
+		{
+			var subscriber = new EventSubscriber();
+			publisher.Event += subscriber.M;
+			weakRefs.Add(new WeakReference<EventSubscriber>(subscriber));
+		}
+	}
+}
+#endif

src/Uno.UI/Helpers/WeakEvents/WeakEventManager.cs

@@ -4,139 +4,135 @@
 using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Reflection;
+using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 
-// Mostly from https://github.com/dotnet/maui/blob/c92d44c68fe81c57ef8caec2506e6788309b8ff4/src/Core/src/WeakEventManager.cs
-// But adjusted a little bit
+namespace Uno.UI.Helpers;
 
-namespace Uno.UI.Helpers
+internal sealed class WeakEventManager
 {
-	internal sealed class WeakEventManager
+	private readonly ConditionalWeakTable<object, Dictionary<string, List<Action>>> _handlersPerTarget = new();
+	private Dictionary<string, List<Action>>? _staticHandlers;
+	private string? _enumeratingHandlersOfEvent;
+	private Stack<string>? _enumeratingHandlersOfEventStack;
+
+	public void AddEventHandler(Action? handler, [CallerMemberName] string eventName = "")
 	{
-		private readonly Dictionary<string, List<Subscription>> _eventHandlers = new(StringComparer.Ordinal);
+		ArgumentException.ThrowIfNullOrEmpty(eventName);
 
-		public void AddEventHandler(Action? handler, [CallerMemberName] string eventName = "")
+		if (handler is null)
 		{
-			if (string.IsNullOrEmpty(eventName))
-				throw new ArgumentNullException(nameof(eventName));
-
-			if (handler == null)
-				throw new ArgumentNullException(nameof(handler));
-
-			AddEventHandler(eventName, handler.Target, handler.GetMethodInfo());
+			// Allow handler to be nullable to not have to deal with nullability checks in callsites, but this indicates a bug.
+			Debug.Fail($"Got a null event handler for event {eventName}.");
+			return;
 		}
 
-		public void HandleEvent(string eventName)
+		var target = handler.Target;
+		if (target is null)
 		{
-			if (_eventHandlers.TryGetValue(eventName, out List<Subscription>? target))
-			{
-				// clone the target array just in case one of the subscriptions calls RemoveEventHandler
-				var targetClone = ArrayPool<Subscription>.Shared.Rent(target.Count);
-				target.CopyTo(targetClone, 0);
-				var count = target.Count;
-				for (int i = 0; i < count; i++)
-				{
-					Subscription subscription = targetClone[i];
-					bool isStatic = subscription.Subscriber == null;
-					if (isStatic)
-					{
-						// For a static method, we'll just pass null as the first parameter of MethodInfo.Invoke
-						subscription.Handler.Invoke(null, null);
-						continue;
-					}
-
-					object? subscriber = subscription.Subscriber?.Target;
-
-					if (subscriber == null)
-					{
-						// The subscriber was collected, so there's no need to keep this subscription around
-						target.Remove(subscription);
-					}
-					else
-					{
-						subscription.Handler.Invoke(subscriber, null);
-					}
-				}
-
-				ArrayPool<Subscription>.Shared.Return(targetClone);
-			}
+			AddTo(_staticHandlers ??= new(), eventName, handler);
 		}
-
-		public void RemoveEventHandler(Action? handler, [CallerMemberName] string eventName = "")
+		else
 		{
-			if (string.IsNullOrEmpty(eventName))
-				throw new ArgumentNullException(nameof(eventName));
+			AddTo(_handlersPerTarget.GetValue(target, static _ => new()), eventName, handler);
+		}
+	}
 
-			if (handler == null)
-				throw new ArgumentNullException(nameof(handler));
+	public void RemoveEventHandler(Action? handler, [CallerMemberName] string eventName = "")
+	{
+		ArgumentException.ThrowIfNullOrEmpty(eventName);
 
-			RemoveEventHandler(eventName, handler.Target, handler.GetMethodInfo());
+		if (handler is null)
+		{
+			// Allow handler to be nullable to not have to deal with nullability checks in callsites, but this indicates a bug.
+			Debug.Fail($"Got a null event handler for event {eventName}.");
+			return;
 		}
 
-		private void AddEventHandler(string eventName, object? handlerTarget, MethodInfo methodInfo)
+		var target = handler.Target;
+		if (target is null)
 		{
-			if (!_eventHandlers.TryGetValue(eventName, out List<Subscription>? targets))
-			{
-				targets = new List<Subscription>();
-				_eventHandlers.Add(eventName, targets);
-			}
-			else
+			if (_staticHandlers is not null)
 			{
-				targets.RemoveAll(subscription => subscription.Subscriber is { IsAlive: false });
+				RemoveFrom(_staticHandlers, eventName, handler);
 			}
-
-			if (handlerTarget == null)
-			{
-				// This event handler is a static method
-				targets.Add(new Subscription(null, methodInfo));
-				return;
-			}
-
-			targets.Add(new Subscription(new WeakReference(handlerTarget), methodInfo));
 		}
+		else if (_handlersPerTarget.TryGetValue(target, out var handlersPerEvent))
+		{
+			RemoveFrom(handlersPerEvent, eventName, handler);
+		}
+	}
 
-		private void RemoveEventHandler(string eventName, object? handlerTarget, MethodInfo methodInfo)
+	public void HandleEvent(string eventName)
+	{
+		if (_enumeratingHandlersOfEvent is not null)
 		{
-			if (!_eventHandlers.TryGetValue(eventName, out List<Subscription>? subscriptions))
-				return;
+			_enumeratingHandlersOfEventStack ??= new();
+			_enumeratingHandlersOfEventStack.Push(_enumeratingHandlersOfEvent);
+		}
+		_enumeratingHandlersOfEvent = eventName;
 
-			for (int n = subscriptions.Count - 1; n >= 0; n--)
+		try
+		{
+			foreach (var kvp in _handlersPerTarget)
 			{
-				Subscription current = subscriptions[n];
-
-				if (current.Subscriber != null && !current.Subscriber.IsAlive)
+				if (!kvp.Value.TryGetValue(eventName, out var handlers))
 				{
-					// If not alive, remove and continue
-					subscriptions.RemoveAt(n);
+					// Event handlers not found for this target
 					continue;
 				}
 
-				if (current.Subscriber?.Target == handlerTarget && current.Handler == methodInfo)
+				var count = handlers.Count;
+				for (var i = 0; i < count; i++)
 				{
-					// Found the match, we can break
-					subscriptions.RemoveAt(n);
-					break;
+					handlers[i]();
 				}
 			}
 		}
+		finally
+		{
+			_enumeratingHandlersOfEvent = (_enumeratingHandlersOfEventStack?.TryPop(out var previous) ?? false) ? previous : null;
+		}
+	}
 
-		private readonly struct Subscription : IEquatable<Subscription>
+	private void AddTo(Dictionary<string, List<Action>> handlersPerEvent, string eventName, Action handler)
+	{
+		ref var handlers = ref CollectionsMarshal.GetValueRefOrAddDefault(handlersPerEvent, eventName, out var exists);
+		if (exists)
 		{
-			public Subscription(WeakReference? subscriber, MethodInfo handler)
+			if (_enumeratingHandlersOfEvent == eventName)
 			{
-				Subscriber = subscriber;
-				Handler = handler ?? throw new ArgumentNullException(nameof(handler));
+#if NET8_0_OR_GREATER
+				handlers = handlers![..];
+#else
+				handlers = handlers!.ToList();
+#endif
 			}
 
-			public readonly WeakReference? Subscriber;
-			public readonly MethodInfo Handler;
-
-			public bool Equals(Subscription other) => Subscriber == other.Subscriber && Handler == other.Handler;
+			handlers!.Add(handler);
+		}
+		else
+		{
+			handlers = [handler];
+		}
+	}
 
-			public override bool Equals(object? obj) => obj is Subscription other && Equals(other);
+	private void RemoveFrom(Dictionary<string, List<Action>> handlersPerEvent, string eventName, Action handler)
+	{
+		ref var handlers = ref CollectionsMarshal.GetValueRefOrNullRef(handlersPerEvent, eventName);
+		if (!Unsafe.IsNullRef(ref handlers))
+		{
+			if (_enumeratingHandlersOfEvent == eventName)
+			{
+#if NET8_0_OR_GREATER
+				handlers = handlers[..];
+#else
+				handlers = handlers!.ToList();
+#endif
+			}
 
-			public override int GetHashCode() => Subscriber?.GetHashCode() ?? 0 ^ Handler.GetHashCode();
+			handlers.Remove(handler);
 		}
 	}
 }