Browse Source

Fix event handler leak in ChildUnitOfWork

ChildUnitOfWork subscribed a lambda to the parent's Failed and Disposed
events in its constructor and never unsubscribed, so every nested unit of
work left two delegates on the parent for the parent's whole lifetime.
Each delegate also captured the child, keeping disposed child instances
alive.

Forward the Failed and Disposed events straight to the parent with event
accessors instead, so nothing accumulates on the parent.

Handlers attached to a child still receive the parent's events with the
same sender and arguments, but their relative order can change: a handler
subscribed through a child now runs at the position where it was
subscribed, rather than where the child was constructed.
pull/26122/head
Eyüpcan Çakır 2 days ago
parent
commit
5a55a1d781
  1. 18
      framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs
  2. 68
      framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Child_Events_Tests.cs

18
framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs

@ -22,8 +22,19 @@ internal class ChildUnitOfWork : IUnitOfWork
public string? ReservationName => _parent.ReservationName;
public event EventHandler<UnitOfWorkFailedEventArgs> Failed = default!;
public event EventHandler<UnitOfWorkEventArgs> Disposed = default!;
// Forwarded directly to the parent, so a child does not have to subscribe
// (and leak) a handler on the parent for its own lifetime.
public event EventHandler<UnitOfWorkFailedEventArgs> Failed
{
add => _parent.Failed += value;
remove => _parent.Failed -= value;
}
public event EventHandler<UnitOfWorkEventArgs> Disposed
{
add => _parent.Disposed += value;
remove => _parent.Disposed -= value;
}
public IServiceProvider ServiceProvider => _parent.ServiceProvider;
@ -38,9 +49,6 @@ internal class ChildUnitOfWork : IUnitOfWork
_parent = parent;
_parent.IncrementActiveChildUnitOfWorkCount();
_parent.Failed += (sender, args) => { Failed.InvokeSafely(sender!, args); };
_parent.Disposed += (sender, args) => { Disposed.InvokeSafely(sender!, args); };
}
public void SetOuter(IUnitOfWork? outer)

68
framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Child_Events_Tests.cs

@ -0,0 +1,68 @@
using System;
using System.Reflection;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using Volo.Abp.Testing;
using Xunit;
namespace Volo.Abp.Uow;
public class UnitOfWork_Child_Events_Tests : AbpIntegratedTest<AbpUnitOfWorkModule>
{
private readonly IUnitOfWorkManager _unitOfWorkManager;
public UnitOfWork_Child_Events_Tests()
{
_unitOfWorkManager = ServiceProvider.GetRequiredService<IUnitOfWorkManager>();
}
[Fact]
public void Child_UnitOfWorks_Should_Not_Accumulate_Event_Handlers_On_The_Parent()
{
using (var parentUow = _unitOfWorkManager.Begin())
{
var failedHandlerCount = GetEventHandlerCount(parentUow, nameof(IUnitOfWork.Failed));
var disposedHandlerCount = GetEventHandlerCount(parentUow, nameof(IUnitOfWork.Disposed));
for (var i = 0; i < 100; i++)
{
using (var childUow = _unitOfWorkManager.Begin())
{
childUow.Id.ShouldBe(parentUow.Id); //It's a child of the parent UOW.
}
}
//Disposed child UOWs should not leave any handler behind on the parent.
GetEventHandlerCount(parentUow, nameof(IUnitOfWork.Failed)).ShouldBe(failedHandlerCount);
GetEventHandlerCount(parentUow, nameof(IUnitOfWork.Disposed)).ShouldBe(disposedHandlerCount);
}
}
[Fact]
public void Should_Trigger_Disposed_Event_Subscribed_Over_A_Child_UnitOfWork()
{
var disposed = false;
using (var parentUow = _unitOfWorkManager.Begin())
{
using (var childUow = _unitOfWorkManager.Begin())
{
childUow.Disposed += (sender, args) => disposed = true;
}
disposed.ShouldBeFalse(); //The parent UOW has not been disposed yet!
}
disposed.ShouldBeTrue();
}
private static int GetEventHandlerCount(IUnitOfWork unitOfWork, string eventName)
{
var field = unitOfWork.GetType().GetField(eventName, BindingFlags.Instance | BindingFlags.NonPublic);
field.ShouldNotBeNull();
return field.GetValue(unitOfWork) is Delegate handler
? handler.GetInvocationList().Length
: 0;
}
}
Loading…
Cancel
Save