diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index 1788cb24139..29c53389c40 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -159,6 +159,7 @@ Polly.PredicateBuilder.PredicateBuilder() -> void Polly.PredicateResult Polly.Registry.ConfigureBuilderContext Polly.Registry.ConfigureBuilderContext.EnableReloads(System.Func!>! tokenProducerFactory) -> void +Polly.Registry.ConfigureBuilderContext.OnPipelineDisposed(System.Action! callback) -> void Polly.Registry.ConfigureBuilderContext.PipelineKey.get -> TKey Polly.Registry.ResiliencePipelineProvider Polly.Registry.ResiliencePipelineProvider.ResiliencePipelineProvider() -> void diff --git a/src/Polly.Core/Registry/ConfigureBuilderContext.cs b/src/Polly.Core/Registry/ConfigureBuilderContext.cs index 6084c2e2160..4edf96c6014 100644 --- a/src/Polly.Core/Registry/ConfigureBuilderContext.cs +++ b/src/Polly.Core/Registry/ConfigureBuilderContext.cs @@ -33,6 +33,8 @@ internal ConfigureBuilderContext(TKey strategyKey, string builderName, string? b internal Func>? ReloadTokenProducer { get; private set; } + internal List DisposeCallbacks { get; } = new(); + /// /// Enables dynamic reloading of the strategy retrieved from . /// @@ -48,4 +50,15 @@ public void EnableReloads(Func> tokenProducerFactory) ReloadTokenProducer = tokenProducerFactory; } + + /// + /// Registers a callback that is called when the pipeline instance being configured is disposed. + /// + /// The callback delegate. + public void OnPipelineDisposed(Action callback) + { + Guard.NotNull(callback); + + DisposeCallbacks.Add(callback); + } } diff --git a/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs b/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs index fc4bf8a0a13..17431e37d2e 100644 --- a/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs +++ b/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs @@ -60,13 +60,17 @@ private Builder CreateBuilder() _configure(builder, context); return new( - builder.BuildPipelineComponent, + () => PipelineComponentFactory.WithDisposableCallbacks( + builder.BuildPipelineComponent(), + context.DisposeCallbacks), context.ReloadTokenProducer, + context.DisposeCallbacks, builder.TelemetryListener); } private record Builder( Func ComponentFactory, Func>? ReloadTokenProducer, + List DisposeCallbacks, TelemetryListener? Listener); } diff --git a/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs b/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs new file mode 100644 index 00000000000..e8a205af4c5 --- /dev/null +++ b/src/Polly.Core/Utils/Pipeline/ComponentWithDisposeCallbacks.cs @@ -0,0 +1,43 @@ +namespace Polly.Utils.Pipeline; + +internal class ComponentWithDisposeCallbacks : PipelineComponent +{ + private readonly List _callbacks; + + public ComponentWithDisposeCallbacks(PipelineComponent component, List callbacks) + { + Component = component; + _callbacks = callbacks; + } + + internal PipelineComponent Component { get; } + + public override void Dispose() + { + ExecuteCallbacks(); + + Component.Dispose(); + } + + public override ValueTask DisposeAsync() + { + ExecuteCallbacks(); + + return Component.DisposeAsync(); + } + + internal override ValueTask> ExecuteCore( + Func>> callback, + ResilienceContext context, + TState state) => Component.ExecuteCore(callback, context, state); + + private void ExecuteCallbacks() + { + foreach (var callback in _callbacks) + { + callback(); + } + + _callbacks.Clear(); + } +} diff --git a/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs b/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs index d5473d442ad..3818b027f99 100644 --- a/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs +++ b/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Polly.Telemetry; + namespace Polly.Utils.Pipeline; internal static class PipelineComponentFactory @@ -13,6 +14,16 @@ internal static class PipelineComponentFactory public static PipelineComponent FromStrategy(ResilienceStrategy strategy) => new BridgeComponent(strategy); + public static PipelineComponent WithDisposableCallbacks(PipelineComponent component, IEnumerable callbacks) + { + if (!callbacks.Any()) + { + return component; + } + + return new ComponentWithDisposeCallbacks(component, callbacks.ToList()); + } + public static PipelineComponent CreateComposite( IReadOnlyList components, ResilienceStrategyTelemetry telemetry, diff --git a/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs b/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs index f2fd5f4c480..cd7b8e98d06 100644 --- a/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs +++ b/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Polly.Registry; +using Polly.Utils; namespace Polly.DependencyInjection; @@ -63,4 +64,15 @@ internal AddResiliencePipelineContext(ConfigureBuilderContext registryCont return name == null ? monitor.CurrentValue : monitor.Get(name); } + + /// + /// Registers a callback that is called when the pipeline instance being configured is disposed. + /// + /// The callback delegate. + public void OnPipelineDisposed(Action callback) + { + Guard.NotNull(callback); + + RegistryContext.OnPipelineDisposed(callback); + } } diff --git a/src/Polly.Extensions/PublicAPI.Unshipped.txt b/src/Polly.Extensions/PublicAPI.Unshipped.txt index f33a3debed8..ec9ab693b79 100644 --- a/src/Polly.Extensions/PublicAPI.Unshipped.txt +++ b/src/Polly.Extensions/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ abstract Polly.Telemetry.MeteringEnricher.Enrich(in Polly.Teleme Polly.DependencyInjection.AddResiliencePipelineContext Polly.DependencyInjection.AddResiliencePipelineContext.EnableReloads(string? name = null) -> void Polly.DependencyInjection.AddResiliencePipelineContext.GetOptions(string? name = null) -> TOptions +Polly.DependencyInjection.AddResiliencePipelineContext.OnPipelineDisposed(System.Action! callback) -> void Polly.DependencyInjection.AddResiliencePipelineContext.PipelineKey.get -> TKey Polly.DependencyInjection.AddResiliencePipelineContext.ServiceProvider.get -> System.IServiceProvider! Polly.PollyServiceCollectionExtensions diff --git a/src/Polly.Testing/ResiliencePipelineExtensions.cs b/src/Polly.Testing/ResiliencePipelineExtensions.cs index db5d03589b8..0435edcc5c9 100644 --- a/src/Polly.Testing/ResiliencePipelineExtensions.cs +++ b/src/Polly.Testing/ResiliencePipelineExtensions.cs @@ -62,7 +62,7 @@ private static object GetStrategyInstance(PipelineComponent component) return component; } - private static bool ShouldSkip(object instance) => instance is ReloadableComponent; + private static bool ShouldSkip(object instance) => instance is ReloadableComponent || instance is ComponentWithDisposeCallbacks; private static void ExpandComponents(this PipelineComponent component, List components) { @@ -78,6 +78,10 @@ private static void ExpandComponents(this PipelineComponent component, List(); + using var changeSource = new CancellationTokenSource(); + var disposedCalls = 0; + + registry.TryAddBuilder("dummy", (builder, context) => + { + // this call enables dynamic reloads for the dummy strategy + context.EnableReloads(() => () => changeSource.Token); + context.OnPipelineDisposed(() => disposedCalls++); + builder.AddTimeout(TimeSpan.FromSeconds(1)); + }); + + // act + var strategy = registry.GetPipeline("dummy"); + + // assert + disposedCalls.Should().Be(0); + strategy.Execute(() => { }); + + changeSource.Cancel(); + disposedCalls.Should().Be(1); + strategy.Execute(() => { }); + + registry.Dispose(); + disposedCalls.Should().Be(2); + } + [Fact] public void EnableReloads_Generic_Ok() { diff --git a/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs new file mode 100644 index 00000000000..a745a54de48 --- /dev/null +++ b/test/Polly.Core.Tests/Utils/Pipeline/ComponentWithDisposeCallbacksTests.cs @@ -0,0 +1,46 @@ +using NSubstitute; +using Polly.Utils.Pipeline; + +namespace Polly.Core.Tests.Utils.Pipeline; + +public class ComponentWithDisposeCallbacksTests +{ + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task Dispose_Ok(bool isAsync) + { + // Arrange + var called1 = 0; + var called2 = 0; + + var callbacks = new List + { + () => called1++, + () => called2++ + }; + var component = Substitute.For(); + var sut = new ComponentWithDisposeCallbacks(component, callbacks); + + // Act + if (isAsync) + { + await sut.DisposeAsync(); + await sut.DisposeAsync(); + await component.Received(2).DisposeAsync(); + } + else + { + sut.Dispose(); +#pragma warning disable S3966 // Objects should not be disposed more than once + sut.Dispose(); +#pragma warning restore S3966 // Objects should not be disposed more than once + component.Received(2).Dispose(); + } + + // Assert + callbacks.Should().BeEmpty(); + called1.Should().Be(1); + called2.Should().Be(1); + } +} diff --git a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs new file mode 100644 index 00000000000..5beed7f1567 --- /dev/null +++ b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs @@ -0,0 +1,26 @@ +using NSubstitute; +using Polly.Utils.Pipeline; + +namespace Polly.Core.Tests.Utils.Pipeline; + +public class PipelineComponentFactoryTests +{ + [Fact] + public void WithDisposableCallbacks_NoCallbacks_ReturnsOriginalComponent() + { + var component = Substitute.For(); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, new List()); + result.Should().BeSameAs(component); + } + + [Fact] + public void PipelineComponentFactory_Should_Return_WrapperComponent_With_Callbacks() + { + var component = Substitute.For(); + var callbacks = new List { () => { } }; + + var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks); + + result.Should().BeOfType(); + } +} diff --git a/test/Polly.Extensions.Tests/DisposablePipelineTests.cs b/test/Polly.Extensions.Tests/DisposablePipelineTests.cs new file mode 100644 index 00000000000..90c5b8b2fe2 --- /dev/null +++ b/test/Polly.Extensions.Tests/DisposablePipelineTests.cs @@ -0,0 +1,58 @@ +using System.Threading.RateLimiting; +using Microsoft.Extensions.DependencyInjection; +using Polly.RateLimiting; +using Polly.Registry; + +namespace Polly.Extensions.Tests; + +public class DisposablePipelineTests +{ + [Fact] + public void DisposePipeline_EnsureLinkedResourcesDisposedToo() + { + var limiters = new List(); + + var provider = new ServiceCollection() + .AddResiliencePipeline("my-pipeline", (builder, context) => + { + var limiter = new ConcurrencyLimiter(new ConcurrencyLimiterOptions + { + PermitLimit = 1, + QueueLimit = 1 + }); + limiters.Add(limiter); + + builder.AddRateLimiter(new RateLimiterStrategyOptions + { + RateLimiter = args => limiter.AcquireAsync(1, args.Context.CancellationToken) + }); + + // when the pipeline instance is disposed, limiter is disposed too + context.OnPipelineDisposed(() => limiter.Dispose()); + }) + .BuildServiceProvider(); + + limiters.Should().HaveCount(0); + provider.GetRequiredService>().GetPipeline("my-pipeline"); + provider.GetRequiredService>().GetPipeline("my-pipeline"); + limiters.Should().HaveCount(1); + IsDisposed(limiters[0]).Should().BeFalse(); + + provider.Dispose(); + limiters.Should().HaveCount(1); + IsDisposed(limiters[0]).Should().BeTrue(); + } + + private static bool IsDisposed(RateLimiter limiter) + { + try + { + limiter.AcquireAsync(1).AsTask().GetAwaiter().GetResult(); + return false; + } + catch (ObjectDisposedException) + { + return true; + } + } +} diff --git a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs index b27568ae4c8..b609337e93a 100644 --- a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs +++ b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs @@ -116,7 +116,7 @@ public void GetPipelineDescriptor_Reloadable_Ok() var strategy = registry.GetOrAddPipeline("dummy", (builder, context) => { context.EnableReloads(() => () => CancellationToken.None); - + context.OnPipelineDisposed(() => { }); builder .AddConcurrencyLimiter(10) .AddStrategy(_ => new CustomStrategy(), new TestOptions());