From 4be9ddd3013a2a00d37c69f821a0b1a7cb9ea6de Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Mon, 25 Nov 2024 15:23:31 -0500 Subject: [PATCH] chore: Keep the delegate active, freeze invocation collection --- .../Tests/Windows_UI/Given_WeakEventHelper.cs | 2 +- src/Uno.UWP/UI/Core/WeakEventHelper.cs | 38 +++++++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs index 11bc0084cdcb..2d33b095c8f7 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI/Given_WeakEventHelper.cs @@ -95,7 +95,7 @@ void Do() SUT.Invoke(this, null); - Assert.AreEqual(1, invoked); + Assert.AreEqual(2, invoked); } [TestMethod] diff --git a/src/Uno.UWP/UI/Core/WeakEventHelper.cs b/src/Uno.UWP/UI/Core/WeakEventHelper.cs index 806ef896a463..d60eaff42feb 100644 --- a/src/Uno.UWP/UI/Core/WeakEventHelper.cs +++ b/src/Uno.UWP/UI/Core/WeakEventHelper.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Runtime.InteropServices; using System.Text; using Uno.Buffers; using Uno.Disposables; @@ -94,9 +95,27 @@ public void Invoke(object sender, object? args) { lock (_lock) { - for (int i = 0; i < _handlers.Count; i++) + if (_handlers.Count == 1) { - _handlers[i].Handler(sender, args); + // Fast path for single registrations + var handler = _handlers[0]; + handler.Handler(sender, args); + } + else if (_handlers.Count > 1) + { + // Clone the array to account for reentrancy. + var handlers = ArrayPool.Shared.Rent(_handlers.Count); + _handlers.CopyTo(handlers, 0); + + // Use the original count, the rented array may be larger. + var count = _handlers.Count; + + for (int i = 0; i < count; i++) + { + handlers[i].Handler(sender, args); + } + + ArrayPool.Shared.Return(handlers); } } } @@ -105,8 +124,8 @@ public void Invoke(object sender, object? args) /// Do not use directly, use instead. /// Registers an handler to be called when the event is raised. /// - /// A disposable that can be used to unregister the provided handler. The disposable instance is not tracked by the GC and will not collect the registration. - internal IDisposable Register(WeakReference target, GenericEventHandler handler) + /// A disposable that can be used to unregister the provided handler. + internal IDisposable Register(WeakReference target, GenericEventHandler handler, Delegate actualTarget) { lock (_lock) { @@ -122,6 +141,13 @@ internal IDisposable Register(WeakReference target, GenericEventHandler handler) void RemoveRegistration() { + // Force a capture of the delegate by setting it to null. + // This will ensure that keeping the IDisposable alive will + // keep the registered original delegate alive. This does ensure + // that if the original delegate was provided as a lambda, the + // instance will stay active as long as the disposable instance. + actualTarget = null!; + lock (_lock) { _handlers.Remove(key); @@ -154,8 +180,6 @@ public void Dispose() /// /// If either the or the are /// collected, raising the event will produce nothing. - /// - /// The returned disposable instance itself is not tracked by the GC and will not collect the registration. /// internal static IDisposable RegisterEvent(WeakEventCollection list, Delegate handler, EventRaiseHandler raise) { @@ -178,7 +202,7 @@ internal static IDisposable RegisterEvent(WeakEventCollection list, Delegate han } }; - return list.Register(wr, genericHandler); + return list.Register(wr, genericHandler, handler); } } }