Skip to content

Commit

Permalink
chore: Keep the delegate active, freeze invocation collection
Browse files Browse the repository at this point in the history
  • Loading branch information
jeromelaban committed Nov 25, 2024
1 parent 6ca6b65 commit 4be9ddd
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void Do()

SUT.Invoke(this, null);

Assert.AreEqual(1, invoked);
Assert.AreEqual(2, invoked);
}

[TestMethod]
Expand Down
38 changes: 31 additions & 7 deletions src/Uno.UWP/UI/Core/WeakEventHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Text;
using Uno.Buffers;
using Uno.Disposables;
Expand Down Expand Up @@ -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<WeakHandler>.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<WeakHandler>.Shared.Return(handlers);
}
}
}
Expand All @@ -105,8 +124,8 @@ public void Invoke(object sender, object? args)
/// Do not use directly, use <see cref="RegisterEvent"/> instead.
/// Registers an handler to be called when the event is raised.
/// </summary>
/// <returns>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.</returns>
internal IDisposable Register(WeakReference target, GenericEventHandler handler)
/// <returns>A disposable that can be used to unregister the provided handler.</returns>
internal IDisposable Register(WeakReference target, GenericEventHandler handler, Delegate actualTarget)
{
lock (_lock)
{
Expand All @@ -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!;

Check notice on line 149 in src/Uno.UWP/UI/Core/WeakEventHelper.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UWP/UI/Core/WeakEventHelper.cs#L149

Introduce a new variable instead of reusing the parameter 'actualTarget'.

lock (_lock)
{
_handlers.Remove(key);
Expand Down Expand Up @@ -154,8 +180,6 @@ public void Dispose()
///
/// If either the <paramref name="list"/> or the <paramref name="handler"/> 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.
/// </remarks>
internal static IDisposable RegisterEvent(WeakEventCollection list, Delegate handler, EventRaiseHandler raise)
{
Expand All @@ -178,7 +202,7 @@ internal static IDisposable RegisterEvent(WeakEventCollection list, Delegate han
}
};

return list.Register(wr, genericHandler);
return list.Register(wr, genericHandler, handler);
}
}
}

0 comments on commit 4be9ddd

Please sign in to comment.