From 5a55a1d781ccb33e6bf5cd7abd00f1aba879b116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCpcan=20=C3=87ak=C4=B1r?= <69207222+eypcnckr@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:05:29 +0300 Subject: [PATCH] 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. --- .../Volo/Abp/Uow/ChildUnitOfWork.cs | 18 +++-- .../Abp/Uow/UnitOfWork_Child_Events_Tests.cs | 68 +++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Child_Events_Tests.cs diff --git a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs b/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs index 14625aaf4a..3451a308cd 100644 --- a/framework/src/Volo.Abp.Uow/Volo/Abp/Uow/ChildUnitOfWork.cs +++ b/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 Failed = default!; - public event EventHandler 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 Failed + { + add => _parent.Failed += value; + remove => _parent.Failed -= value; + } + + public event EventHandler 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) diff --git a/framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Child_Events_Tests.cs b/framework/test/Volo.Abp.Uow.Tests/Volo/Abp/Uow/UnitOfWork_Child_Events_Tests.cs new file mode 100644 index 0000000000..faa8d8ef61 --- /dev/null +++ b/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 +{ + private readonly IUnitOfWorkManager _unitOfWorkManager; + + public UnitOfWork_Child_Events_Tests() + { + _unitOfWorkManager = ServiceProvider.GetRequiredService(); + } + + [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; + } +}