diff --git a/src/Core/src/WeakEventManager.cs b/src/Core/src/WeakEventManager.cs index 413c586f7ed9..1bc2f1a07a00 100644 --- a/src/Core/src/WeakEventManager.cs +++ b/src/Core/src/WeakEventManager.cs @@ -129,11 +129,19 @@ void RemoveEventHandler(string eventName, object? handlerTarget, MemberInfo meth { Subscription current = subscriptions[n - 1]; - if (current.Subscriber?.Target != handlerTarget || current.Handler.Name != methodInfo.Name) + if (current.Subscriber != null && !current.Subscriber.IsAlive) + { + // If not alive, remove and continue + subscriptions.RemoveAt(n); continue; + } - subscriptions.Remove(current); - break; + if (current.Subscriber?.Target == handlerTarget && current.Handler.Name == methodInfo.Name) + { + // Found the match, we can break + subscriptions.RemoveAt(n); + break; + } } } diff --git a/src/Core/tests/UnitTests/WeakEventManagerTests.cs b/src/Core/tests/UnitTests/WeakEventManagerTests.cs index da3992d36a00..656ab09362ed 100644 --- a/src/Core/tests/UnitTests/WeakEventManagerTests.cs +++ b/src/Core/tests/UnitTests/WeakEventManagerTests.cs @@ -1,6 +1,9 @@ #nullable enable using System; +using System.Collections; using System.ComponentModel; +using System.Linq; +using System.Reflection; using Xunit; namespace Microsoft.Maui.UnitTests @@ -65,6 +68,19 @@ void OnTestEvent() { _weakEventManager.HandleEvent(this, EventArgs.Empty, nameof(TestEvent)); } + + public int EventHandlerCount + { + get + { + // Access private members: + // Dictionary> eventHandlers = WeakEventManager._eventHandlers; + var flags = BindingFlags.NonPublic | BindingFlags.Instance; + var eventHandlers = _weakEventManager.GetType().GetField("_eventHandlers", flags)?.GetValue(_weakEventManager) as IDictionary; + Assert.NotNull(eventHandlers); + return eventHandlers.Values.Cast().Sum(l => l.Count); + } + } } internal class TestSubscriber @@ -74,6 +90,11 @@ public void Subscribe(TestEventSource source) source.TestEvent += SourceOnTestEvent; } + public void Unsubscribe(TestEventSource source) + { + source.TestEvent -= SourceOnTestEvent; + } + void SourceOnTestEvent(object? sender, EventArgs eventArgs) { throw new Exception("Fail"); @@ -230,7 +251,7 @@ public void StaticHandlerShouldRun() } [Fact] - public void VerifySubscriberCanBeCollected() + public void VerifySubscriberCanBeCollected_FireEvent() { WeakReference? wr = null; var source = new TestEventSource(); @@ -250,6 +271,28 @@ public void VerifySubscriberCanBeCollected() // The handler for this calls Assert.Fail, so if the subscriber has not been collected // the handler will be called and the test will fail source.FireTestEvent(); + Assert.Equal(0, source.EventHandlerCount); + } + + [Fact] + public void VerifySubscriberCanBeCollected_Unsubscribe() + { + WeakReference? wr = null; + var source = new TestEventSource(); + new Action(() => + { + var ts = new TestSubscriber(); + wr = new WeakReference(ts); + ts.Subscribe(source); + })(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + new TestSubscriber().Unsubscribe(source); + + Assert.NotNull(wr); + Assert.False(wr?.IsAlive); + Assert.Equal(0, source.EventHandlerCount); } [Fact]