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 1/2] 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; + } +} From 374f9c53613b15ad3195e918e5122e09c2c46b15 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:30 +0300 Subject: [PATCH 2/2] Add an attribute adapter for EnumDataTypeAttribute ASP.NET Core's ValidationAttributeAdapterProvider has no adapter for EnumDataTypeAttribute, so its ErrorMessage is never passed through the IStringLocalizer and the raw localization key is returned to the client, unlike every other common validation attribute. Add EnumDataTypeAttributeAdapter and return it from AbpValidationAttributeAdapterProvider. --- .../AbpValidationAttributeAdapterProvider.cs | 5 ++ .../EnumDataTypeAttributeAdapter.cs | 34 +++++++++ ...alidationAttributeAdapterProvider_Tests.cs | 72 +++++++++++++++++++ 3 files changed, 111 insertions(+) create mode 100644 framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/EnumDataTypeAttributeAdapter.cs create mode 100644 framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider_Tests.cs diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider.cs index 0ba54c5448..5b253b53fe 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider.cs @@ -33,6 +33,11 @@ public class AbpValidationAttributeAdapterProvider : IValidationAttributeAdapter return new DynamicRangeAttributeAdapter((DynamicRangeAttribute)attribute, stringLocalizer); } + if (type == typeof(EnumDataTypeAttribute)) + { + return new EnumDataTypeAttributeAdapter((EnumDataTypeAttribute)attribute, stringLocalizer); + } + return _defaultAdapter.GetAttributeAdapter(attribute, stringLocalizer); } } diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/EnumDataTypeAttributeAdapter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/EnumDataTypeAttributeAdapter.cs new file mode 100644 index 0000000000..29ad39097d --- /dev/null +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/DataAnnotations/EnumDataTypeAttributeAdapter.cs @@ -0,0 +1,34 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Mvc.DataAnnotations; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.Localization; + +namespace Volo.Abp.AspNetCore.Mvc.DataAnnotations; + +public class EnumDataTypeAttributeAdapter : AttributeAdapterBase +{ + public EnumDataTypeAttributeAdapter( + EnumDataTypeAttribute attribute, + IStringLocalizer? stringLocalizer) + : base(attribute, stringLocalizer) + { + } + + public override void AddValidation(ClientModelValidationContext context) + { + Check.NotNull(context, nameof(context)); + + //There is no built-in client side validation rule for enum values. + //This adapter is used to localize the error message on the server side. + } + + public override string GetErrorMessage(ModelValidationContextBase validationContext) + { + Check.NotNull(validationContext, nameof(validationContext)); + + return GetErrorMessage( + validationContext.ModelMetadata, + validationContext.ModelMetadata.GetDisplayName() + ); + } +} diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider_Tests.cs new file mode 100644 index 0000000000..d5bb122939 --- /dev/null +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/DataAnnotations/AbpValidationAttributeAdapterProvider_Tests.cs @@ -0,0 +1,72 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.DataAnnotations; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.Localization; +using Shouldly; +using Xunit; + +namespace Volo.Abp.AspNetCore.Mvc.DataAnnotations; + +public class AbpValidationAttributeAdapterProvider_Tests +{ + private readonly AbpValidationAttributeAdapterProvider _provider = new(new ValidationAttributeAdapterProvider()); + + [Fact] + public void Should_Return_An_Adapter_For_The_EnumDataTypeAttribute() + { + //ASP.NET Core does not provide an adapter for the EnumDataTypeAttribute. + new ValidationAttributeAdapterProvider() + .GetAttributeAdapter(new EnumDataTypeAttribute(typeof(MyEnum)), null) + .ShouldBeNull(); + + _provider.GetAttributeAdapter(new EnumDataTypeAttribute(typeof(MyEnum)), null) + .ShouldBeOfType(); + } + + [Fact] + public void Should_Localize_The_Error_Message_Of_The_EnumDataTypeAttribute() + { + var attribute = new EnumDataTypeAttribute(typeof(MyEnum)) { ErrorMessage = "MyEnumIsInvalid" }; + + var adapter = _provider.GetAttributeAdapter(attribute, new TestStringLocalizer())!; + + adapter.GetErrorMessage(CreateValidationContext()).ShouldBe("Localized:MyEnumIsInvalid"); + } + + private static ModelValidationContextBase CreateValidationContext() + { + var metadataProvider = new EmptyModelMetadataProvider(); + + return new ClientModelValidationContext( + new ActionContext(), + metadataProvider.GetMetadataForProperty(typeof(MyModel), nameof(MyModel.Value)), + metadataProvider, + new Dictionary() + ); + } + + public enum MyEnum + { + Value1 = 1 + } + + public class MyModel + { + public MyEnum Value { get; set; } + } + + private class TestStringLocalizer : IStringLocalizer + { + public LocalizedString this[string name] => new(name, "Localized:" + name); + + public LocalizedString this[string name, params object[] arguments] => new(name, "Localized:" + name); + + public IEnumerable GetAllStrings(bool includeParentCultures) + { + return new List(); + } + } +}