diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs index f618d078a56..bb4adb10a3f 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs @@ -6,9 +6,9 @@ namespace Polly.CircuitBreaker; /// /// The instance of this class can be reused across multiple circuit breakers. /// -public sealed class CircuitBreakerManualControl : IDisposable +public sealed class CircuitBreakerManualControl { - private readonly HashSet _onDispose = new(); + private readonly object _lock = new(); private readonly HashSet> _onIsolate = new(); private readonly HashSet> _onReset = new(); private bool _isolated; @@ -23,21 +23,34 @@ public CircuitBreakerManualControl() /// /// Initializes a new instance of the class. /// - /// Determines whether the circit breaker is isolated immediately after construction. + /// Determines whether the circuit breaker is isolated immediately after construction. public CircuitBreakerManualControl(bool isIsolated) => _isolated = isIsolated; - internal void Initialize(Func onIsolate, Func onReset, Action onDispose) - { - _onDispose.Add(onDispose); - _onIsolate.Add(onIsolate); - _onReset.Add(onReset); + internal bool IsEmpty => _onIsolate.Count == 0; - if (_isolated) + internal IDisposable Initialize(Func onIsolate, Func onReset) + { + lock (_lock) { - var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); - - // if the control indicates that circuit breaker should be isolated, we isolate it right away - IsolateAsync(context).GetAwaiter().GetResult(); + _onIsolate.Add(onIsolate); + _onReset.Add(onReset); + + if (_isolated) + { + var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); + + // if the control indicates that circuit breaker should be isolated, we isolate it right away + IsolateAsync(context).GetAwaiter().GetResult(); + } + + return new RegistrationDisposable(() => + { + lock (_lock) + { + _onIsolate.Remove(onIsolate); + _onReset.Remove(onReset); + } + }); } } @@ -54,7 +67,14 @@ internal async Task IsolateAsync(ResilienceContext context) _isolated = true; - foreach (var action in _onIsolate) + Func[] callbacks; + + lock (_lock) + { + callbacks = _onIsolate.ToArray(); + } + + foreach (var action in callbacks) { await action(context).ConfigureAwait(context.ContinueOnCapturedContext); } @@ -95,7 +115,14 @@ internal async Task CloseAsync(ResilienceContext context) context.Initialize(isSynchronous: false); - foreach (var action in _onReset) + Func[] callbacks; + + lock (_lock) + { + callbacks = _onReset.ToArray(); + } + + foreach (var action in callbacks) { await action(context).ConfigureAwait(context.ContinueOnCapturedContext); } @@ -121,18 +148,12 @@ public async Task CloseAsync(CancellationToken cancellationToken = default) } } - /// - /// Disposes the current class. - /// - public void Dispose() + private class RegistrationDisposable : IDisposable { - foreach (var action in _onDispose) - { - action(); - } + private readonly Action _disposeAction; + + public RegistrationDisposable(Action disposeAction) => _disposeAction = disposeAction; - _onDispose.Clear(); - _onIsolate.Clear(); - _onReset.Clear(); + public void Dispose() => _disposeAction(); } } diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index 5ed65a6a8aa..3ee83d0d47b 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -1,9 +1,10 @@ namespace Polly.CircuitBreaker; -internal sealed class CircuitBreakerResilienceStrategy : ResilienceStrategy +internal sealed class CircuitBreakerResilienceStrategy : ResilienceStrategy, IDisposable { private readonly Func, ValueTask> _handler; private readonly CircuitStateController _controller; + private readonly IDisposable? _manualControlRegistration; public CircuitBreakerResilienceStrategy( Func, ValueTask> handler, @@ -15,10 +16,15 @@ public CircuitBreakerResilienceStrategy( _controller = controller; stateProvider?.Initialize(() => _controller.CircuitState, () => _controller.LastHandledOutcome); - manualControl?.Initialize( + _manualControlRegistration = manualControl?.Initialize( async c => await _controller.IsolateCircuitAsync(c).ConfigureAwait(c.ContinueOnCapturedContext), - async c => await _controller.CloseCircuitAsync(c).ConfigureAwait(c.ContinueOnCapturedContext), - _controller.Dispose); + async c => await _controller.CloseCircuitAsync(c).ConfigureAwait(c.ContinueOnCapturedContext)); + } + + public void Dispose() + { + _manualControlRegistration?.Dispose(); + _controller.Dispose(); } protected internal override async ValueTask> ExecuteCore(Func>> callback, ResilienceContext context, TState state) diff --git a/src/Polly.Core/PublicAPI.Unshipped.txt b/src/Polly.Core/PublicAPI.Unshipped.txt index 0fe492f1c48..c8af645ea81 100644 --- a/src/Polly.Core/PublicAPI.Unshipped.txt +++ b/src/Polly.Core/PublicAPI.Unshipped.txt @@ -24,7 +24,6 @@ Polly.CircuitBreaker.CircuitBreakerManualControl Polly.CircuitBreaker.CircuitBreakerManualControl.CircuitBreakerManualControl() -> void Polly.CircuitBreaker.CircuitBreakerManualControl.CircuitBreakerManualControl(bool isIsolated) -> void Polly.CircuitBreaker.CircuitBreakerManualControl.CloseAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! -Polly.CircuitBreaker.CircuitBreakerManualControl.Dispose() -> void Polly.CircuitBreaker.CircuitBreakerManualControl.IsolateAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Polly.CircuitBreaker.CircuitBreakerPredicateArguments Polly.CircuitBreaker.CircuitBreakerPredicateArguments.CircuitBreakerPredicateArguments() -> void @@ -167,6 +166,8 @@ Polly.Registry.ConfigureBuilderContext.PipelineKey.get -> TKey Polly.Registry.ResiliencePipelineProvider Polly.Registry.ResiliencePipelineProvider.ResiliencePipelineProvider() -> void Polly.Registry.ResiliencePipelineRegistry +Polly.Registry.ResiliencePipelineRegistry.Dispose() -> void +Polly.Registry.ResiliencePipelineRegistry.DisposeAsync() -> System.Threading.Tasks.ValueTask Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action!>! configure) -> Polly.ResiliencePipeline! Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action! configure) -> Polly.ResiliencePipeline! Polly.Registry.ResiliencePipelineRegistry.GetOrAddPipeline(TKey key, System.Action!, Polly.Registry.ConfigureBuilderContext!>! configure) -> Polly.ResiliencePipeline! diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs index 4e3b5668429..5e282e9921f 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs @@ -5,7 +5,7 @@ namespace Polly.Registry; public sealed partial class ResiliencePipelineRegistry : ResiliencePipelineProvider where TKey : notnull { - private sealed class GenericRegistry + private sealed class GenericRegistry : IDisposable, IAsyncDisposable { private readonly Func> _activator; private readonly ConcurrentDictionary, ConfigureBuilderContext>> _builders; @@ -52,14 +52,34 @@ public ResiliencePipeline GetOrAdd(TKey key, Action { - return new ResiliencePipeline(CreatePipelineComponent(factory.instance._activator, factory.context, factory.configure)); + return new ResiliencePipeline(CreatePipelineComponent(factory.instance._activator, factory.context, factory.configure), DisposeBehavior.Reject); }, (instance: this, context, configure)); #else - return _strategies.GetOrAdd(key, _ => new ResiliencePipeline(CreatePipelineComponent(_activator, context, configure))); + return _strategies.GetOrAdd(key, _ => new ResiliencePipeline(CreatePipelineComponent(_activator, context, configure), DisposeBehavior.Reject)); #endif } public bool TryAddBuilder(TKey key, Action, ConfigureBuilderContext> configure) => _builders.TryAdd(key, configure); + + public void Dispose() + { + foreach (var strategy in _strategies.Values) + { + strategy.DisposeHelper.ForceDispose(); + } + + _strategies.Clear(); + } + + public async ValueTask DisposeAsync() + { + foreach (var strategy in _strategies.Values) + { + await strategy.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); + } + + _strategies.Clear(); + } } } diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs index 50a47749107..f8a8bff85ae 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs @@ -16,7 +16,7 @@ namespace Polly.Registry; /// These callbacks are called when the resilience pipeline is not yet cached and it's retrieved for the first time. /// /// -public sealed partial class ResiliencePipelineRegistry : ResiliencePipelineProvider +public sealed partial class ResiliencePipelineRegistry : ResiliencePipelineProvider, IDisposable, IAsyncDisposable where TKey : notnull { private readonly Func _activator; @@ -28,6 +28,7 @@ public sealed partial class ResiliencePipelineRegistry : ResiliencePipelin private readonly Func _builderNameFormatter; private readonly IEqualityComparer _builderComparer; private readonly IEqualityComparer _pipelineComparer; + private bool _disposed; /// /// Initializes a new instance of the class with the default comparer. @@ -63,12 +64,16 @@ public ResiliencePipelineRegistry(ResiliencePipelineRegistryOptions option /// public override bool TryGetPipeline(TKey key, [NotNullWhen(true)] out ResiliencePipeline? pipeline) { + EnsureNotDisposed(); + return GetGenericRegistry().TryGet(key, out pipeline); } /// public override bool TryGetPipeline(TKey key, [NotNullWhen(true)] out ResiliencePipeline? pipeline) { + EnsureNotDisposed(); + if (_pipelines.TryGetValue(key, out pipeline)) { return true; @@ -90,10 +95,13 @@ public override bool TryGetPipeline(TKey key, [NotNullWhen(true)] out Resilience /// The key used to identify the resilience pipeline. /// The callback that configures the pipeline builder. /// An instance of pipeline. + /// Thrown when the registry is already disposed. public ResiliencePipeline GetOrAddPipeline(TKey key, Action configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + return GetOrAddPipeline(key, (builder, _) => configure(builder)); } @@ -103,10 +111,13 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, ActionThe key used to identify the resilience pipeline. /// The callback that configures the pipeline builder. /// An instance of pipeline. + /// Thrown when the registry is already disposed. public ResiliencePipeline GetOrAddPipeline(TKey key, Action> configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + if (_pipelines.TryGetValue(key, out var pipeline)) { return pipeline; @@ -117,11 +128,11 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, Action { - return new ResiliencePipeline(CreatePipelineComponent(factory.instance._activator, factory.context, factory.configure)); + return new ResiliencePipeline(CreatePipelineComponent(factory.instance._activator, factory.context, factory.configure), DisposeBehavior.Reject); }, (instance: this, context, configure)); #else - return _pipelines.GetOrAdd(key, _ => new ResiliencePipeline(CreatePipelineComponent(_activator, context, configure))); + return _pipelines.GetOrAdd(key, _ => new ResiliencePipeline(CreatePipelineComponent(_activator, context, configure), DisposeBehavior.Reject)); #endif } @@ -132,10 +143,13 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, ActionThe key used to identify the resilience pipeline. /// The callback that configures the pipeline builder. /// An instance of pipeline. + /// Thrown when the registry is already disposed. public ResiliencePipeline GetOrAddPipeline(TKey key, Action> configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + return GetOrAddPipeline(key, (builder, _) => configure(builder)); } @@ -146,10 +160,13 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, ActionThe key used to identify the resilience pipeline. /// The callback that configures the pipeline builder. /// An instance of pipeline. + /// Thrown when the registry is already disposed. public ResiliencePipeline GetOrAddPipeline(TKey key, Action, ConfigureBuilderContext> configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + return GetGenericRegistry().GetOrAdd(key, configure); } @@ -163,10 +180,13 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, Action /// Thrown when is . + /// Thrown when the registry is already disposed. public bool TryAddBuilder(TKey key, Action> configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + return _builders.TryAdd(key, configure); } @@ -181,13 +201,66 @@ public bool TryAddBuilder(TKey key, Action /// Thrown when is . + /// Thrown when the registry is already disposed. public bool TryAddBuilder(TKey key, Action, ConfigureBuilderContext> configure) { Guard.NotNull(configure); + EnsureNotDisposed(); + return GetGenericRegistry().TryAddBuilder(key, configure); } + /// + /// Disposes all resources that are held by the resilience pipelines created by this builder. + /// + /// + /// After the disposal, all resilience pipelines still used outside of the builder are disposed + /// and cannot be used anymore. + /// + public void Dispose() + { + _disposed = true; + + var pipelines = _pipelines.Values.ToList(); + _pipelines.Clear(); + + var registries = _genericRegistry.Values.Cast().ToList(); + _genericRegistry.Clear(); + + pipelines.ForEach(p => p.DisposeHelper.ForceDispose()); + registries.ForEach(p => p.Dispose()); + } + + /// + /// Disposes all resources that are held by the resilience pipelines created by this builder. + /// + /// Returns a task that represents the asynchronous dispose operation. + /// + /// After the disposal, all resilience pipelines still used outside of the builder are disposed + /// and cannot be used anymore. + /// + public async ValueTask DisposeAsync() + { + _disposed = true; + + var pipelines = _pipelines.Values.ToList(); + _pipelines.Clear(); + + var registries = _genericRegistry.Values.Cast().ToList(); + _genericRegistry.Clear(); + + foreach (var pipeline in pipelines) + { + await pipeline.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); + } + + foreach (var disposable in registries) + { + await disposable.DisposeAsync().ConfigureAwait(false); + } + } + private static PipelineComponent CreatePipelineComponent( Func activator, ConfigureBuilderContext context, @@ -235,4 +308,12 @@ private GenericRegistry GetGenericRegistry() _instanceNameFormatter); }); } + + private void EnsureNotDisposed() + { + if (_disposed) + { + throw new ObjectDisposedException("ResiliencePipelineRegistry", "The resilience pipeline registry has been disposed and cannot be used anymore."); + } + } } diff --git a/src/Polly.Core/ResiliencePipeline.Async.cs b/src/Polly.Core/ResiliencePipeline.Async.cs index 00ce9bc9577..6e078a44b4c 100644 --- a/src/Polly.Core/ResiliencePipeline.Async.cs +++ b/src/Polly.Core/ResiliencePipeline.Async.cs @@ -164,7 +164,7 @@ static async (context, state) => } } - private static ResilienceContext GetAsyncContext(CancellationToken cancellationToken) => GetAsyncContext(cancellationToken); + private ResilienceContext GetAsyncContext(CancellationToken cancellationToken) => GetAsyncContext(cancellationToken); - private static void InitializeAsyncContext(ResilienceContext context) => InitializeAsyncContext(context); + private void InitializeAsyncContext(ResilienceContext context) => InitializeAsyncContext(context); } diff --git a/src/Polly.Core/ResiliencePipeline.AsyncT.cs b/src/Polly.Core/ResiliencePipeline.AsyncT.cs index 603f0741ba3..2fda3b36e2d 100644 --- a/src/Polly.Core/ResiliencePipeline.AsyncT.cs +++ b/src/Polly.Core/ResiliencePipeline.AsyncT.cs @@ -190,7 +190,7 @@ static async (context, state) => } } - private static ResilienceContext GetAsyncContext(CancellationToken cancellationToken) + private ResilienceContext GetAsyncContext(CancellationToken cancellationToken) { var context = Pool.Get(cancellationToken); @@ -199,5 +199,10 @@ private static ResilienceContext GetAsyncContext(CancellationToken canc return context; } - private static void InitializeAsyncContext(ResilienceContext context) => context.Initialize(isSynchronous: false); + private void InitializeAsyncContext(ResilienceContext context) + { + DisposeHelper.EnsureNotDisposed(); + + context.Initialize(isSynchronous: false); + } } diff --git a/src/Polly.Core/ResiliencePipeline.Sync.cs b/src/Polly.Core/ResiliencePipeline.Sync.cs index ce14a6f430c..909570f218b 100644 --- a/src/Polly.Core/ResiliencePipeline.Sync.cs +++ b/src/Polly.Core/ResiliencePipeline.Sync.cs @@ -225,7 +225,7 @@ public void Execute(Action callback) } } - private static ResilienceContext GetSyncContext(CancellationToken cancellationToken) => GetSyncContext(cancellationToken); + private ResilienceContext GetSyncContext(CancellationToken cancellationToken) => GetSyncContext(cancellationToken); - private static void InitializeSyncContext(ResilienceContext context) => InitializeSyncContext(context); + private void InitializeSyncContext(ResilienceContext context) => InitializeSyncContext(context); } diff --git a/src/Polly.Core/ResiliencePipeline.SyncT.cs b/src/Polly.Core/ResiliencePipeline.SyncT.cs index a3f64aafb91..7c197e921e5 100644 --- a/src/Polly.Core/ResiliencePipeline.SyncT.cs +++ b/src/Polly.Core/ResiliencePipeline.SyncT.cs @@ -231,7 +231,7 @@ public TResult Execute( } } - private static ResilienceContext GetSyncContext(CancellationToken cancellationToken) + private ResilienceContext GetSyncContext(CancellationToken cancellationToken) { var context = Pool.Get(cancellationToken); @@ -240,5 +240,10 @@ private static ResilienceContext GetSyncContext(CancellationToken cance return context; } - private static void InitializeSyncContext(ResilienceContext context) => context.Initialize(isSynchronous: true); + private void InitializeSyncContext(ResilienceContext context) + { + DisposeHelper.EnsureNotDisposed(); + + context.Initialize(isSynchronous: true); + } } diff --git a/src/Polly.Core/ResiliencePipeline.cs b/src/Polly.Core/ResiliencePipeline.cs index ba5c597f5de..2384014c8c0 100644 --- a/src/Polly.Core/ResiliencePipeline.cs +++ b/src/Polly.Core/ResiliencePipeline.cs @@ -12,14 +12,20 @@ public sealed partial class ResiliencePipeline /// /// Resilience pipeline that executes the user-provided callback without any additional logic. /// - public static readonly ResiliencePipeline Null = new(PipelineComponent.Null); + public static readonly ResiliencePipeline Null = new(PipelineComponent.Null, DisposeBehavior.Ignore); - internal ResiliencePipeline(PipelineComponent component) => Component = component; + internal ResiliencePipeline(PipelineComponent component, DisposeBehavior disposeBehavior) + { + Component = component; + DisposeHelper = new ComponentDisposeHelper(component, disposeBehavior); + } internal static ResilienceContextPool Pool => ResilienceContextPool.Shared; internal PipelineComponent Component { get; } + internal ComponentDisposeHelper DisposeHelper { get; } + internal ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, diff --git a/src/Polly.Core/ResiliencePipelineBuilder.TResult.cs b/src/Polly.Core/ResiliencePipelineBuilder.TResult.cs index b6ac86cfa49..ac592e91e74 100644 --- a/src/Polly.Core/ResiliencePipelineBuilder.TResult.cs +++ b/src/Polly.Core/ResiliencePipelineBuilder.TResult.cs @@ -30,5 +30,5 @@ internal ResiliencePipelineBuilder(ResiliencePipelineBuilderBase other) /// /// An instance of . /// Thrown when this builder has invalid configuration. - public ResiliencePipeline Build() => new(BuildPipelineComponent()); + public ResiliencePipeline Build() => new(BuildPipelineComponent(), DisposeBehavior.Allow); } diff --git a/src/Polly.Core/ResiliencePipelineBuilder.cs b/src/Polly.Core/ResiliencePipelineBuilder.cs index 2772fe30a9f..ffca9868231 100644 --- a/src/Polly.Core/ResiliencePipelineBuilder.cs +++ b/src/Polly.Core/ResiliencePipelineBuilder.cs @@ -17,5 +17,5 @@ public sealed class ResiliencePipelineBuilder : ResiliencePipelineBuilderBase /// /// An instance of . /// Thrown when this builder has invalid configuration. - public ResiliencePipeline Build() => new(BuildPipelineComponent()); + public ResiliencePipeline Build() => new(BuildPipelineComponent(), DisposeBehavior.Allow); } diff --git a/src/Polly.Core/ResiliencePipelineBuilderBase.cs b/src/Polly.Core/ResiliencePipelineBuilderBase.cs index 4da5db5bf18..d6e94f8e2f7 100644 --- a/src/Polly.Core/ResiliencePipelineBuilderBase.cs +++ b/src/Polly.Core/ResiliencePipelineBuilderBase.cs @@ -88,7 +88,7 @@ private protected ResiliencePipelineBuilderBase(ResiliencePipelineBuilderBase ot internal Action Validator { get; private protected set; } = ValidationHelper.ValidateObject; [RequiresUnreferencedCode(Constants.OptionsValidation)] - internal void AddStrategyCore(Func factory, ResilienceStrategyOptions options) + internal void AddPipelineComponent(Func factory, ResilienceStrategyOptions options) { Guard.NotNull(factory); Guard.NotNull(options); diff --git a/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs index 04ce0540a44..93ffaf5a946 100644 --- a/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs +++ b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs @@ -27,7 +27,7 @@ public static TBuilder AddPipeline(this TBuilder builder, ResiliencePi Guard.NotNull(builder); Guard.NotNull(pipeline); - builder.AddStrategyCore(_ => PipelineComponent.FromPipeline(pipeline), EmptyOptions.Instance); + builder.AddPipelineComponent(_ => PipelineComponent.FromPipeline(pipeline), EmptyOptions.Instance); return builder; } @@ -49,7 +49,7 @@ public static ResiliencePipelineBuilder AddPipeline(this Resil Guard.NotNull(builder); Guard.NotNull(pipeline); - builder.AddStrategyCore(_ => PipelineComponent.FromPipeline(pipeline), EmptyOptions.Instance); + builder.AddPipelineComponent(_ => PipelineComponent.FromPipeline(pipeline), EmptyOptions.Instance); return builder; } @@ -72,7 +72,7 @@ public static TBuilder AddStrategy(this TBuilder builder, Func PipelineComponent.FromStrategy(factory(context)), options); + builder.AddPipelineComponent(context => PipelineComponent.FromStrategy(factory(context)), options); return builder; } @@ -95,7 +95,7 @@ public static ResiliencePipelineBuilder AddStrategy( Guard.NotNull(factory); Guard.NotNull(options); - builder.AddStrategyCore(context => PipelineComponent.FromStrategy(factory(context)), options); + builder.AddPipelineComponent(context => PipelineComponent.FromStrategy(factory(context)), options); return builder; } @@ -119,7 +119,7 @@ public static ResiliencePipelineBuilder AddStrategy( Guard.NotNull(factory); Guard.NotNull(options); - builder.AddStrategyCore(context => PipelineComponent.FromStrategy(factory(context)), options); + builder.AddPipelineComponent(context => PipelineComponent.FromStrategy(factory(context)), options); return builder; } diff --git a/src/Polly.Core/ResiliencePipelineT.cs b/src/Polly.Core/ResiliencePipelineT.cs index a8cc314ced8..d17030a8718 100644 --- a/src/Polly.Core/ResiliencePipelineT.cs +++ b/src/Polly.Core/ResiliencePipelineT.cs @@ -13,12 +13,20 @@ public sealed partial class ResiliencePipeline /// /// Resilience pipeline that executes the user-provided callback without any additional logic. /// - public static readonly ResiliencePipeline Null = new(PipelineComponent.Null); + public static readonly ResiliencePipeline Null = new(PipelineComponent.Null, DisposeBehavior.Ignore); - internal ResiliencePipeline(PipelineComponent component) => Pipeline = new ResiliencePipeline(component); + internal ResiliencePipeline(PipelineComponent component, DisposeBehavior disposeBehavior) + { + // instead of re-implementing individual Execute* methods we + // can just re-use the non-generic ResiliencePipeline and + // call it from Execute* methods in this class + Pipeline = new ResiliencePipeline(component, disposeBehavior); + DisposeHelper = Pipeline.DisposeHelper; + } internal PipelineComponent Component => Pipeline.Component; - private ResiliencePipeline Pipeline { get; } + internal ComponentDisposeHelper DisposeHelper { get; } + private ResiliencePipeline Pipeline { get; } } diff --git a/src/Polly.Core/Utils/ComponentDisposeHelper.cs b/src/Polly.Core/Utils/ComponentDisposeHelper.cs new file mode 100644 index 00000000000..b4789963096 --- /dev/null +++ b/src/Polly.Core/Utils/ComponentDisposeHelper.cs @@ -0,0 +1,69 @@ +namespace Polly.Utils; + +internal sealed class ComponentDisposeHelper : IDisposable, IAsyncDisposable +{ + private readonly PipelineComponent _component; + private readonly DisposeBehavior _disposeBehavior; + private bool _disposed; + + public ComponentDisposeHelper(PipelineComponent component, DisposeBehavior disposeBehavior) + { + _component = component; + _disposeBehavior = disposeBehavior; + } + + public void Dispose() + { + if (EnsureDisposable()) + { + ForceDispose(); + } + } + + public ValueTask DisposeAsync() + { + if (EnsureDisposable()) + { + return ForceDisposeAsync(); + } + + return default; + } + + public void EnsureNotDisposed() + { + if (_disposed) + { + throw new ObjectDisposedException("ResiliencePipeline", "This resilience pipeline has been disposed and cannot be used anymore."); + } + } + + public void ForceDispose() + { + _disposed = true; +#pragma warning disable S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods + _component.Dispose(); +#pragma warning restore S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods + } + + public ValueTask ForceDisposeAsync() + { + _disposed = true; + return _component.DisposeAsync(); + } + + private bool EnsureDisposable() + { + if (_disposeBehavior == DisposeBehavior.Ignore) + { + return false; + } + + if (_disposeBehavior == DisposeBehavior.Reject) + { + throw new InvalidOperationException("Disposing this resilience pipeline is not allowed because it is owned by the pipeline registry."); + } + + return !_disposed; + } +} diff --git a/src/Polly.Core/Utils/DisposeBehavior.cs b/src/Polly.Core/Utils/DisposeBehavior.cs new file mode 100644 index 00000000000..3ac13cc81c4 --- /dev/null +++ b/src/Polly.Core/Utils/DisposeBehavior.cs @@ -0,0 +1,8 @@ +namespace Polly.Utils; + +internal enum DisposeBehavior +{ + Ignore, + Allow, + Reject +} diff --git a/src/Polly.Core/Utils/PipelineComponent.Bridge.cs b/src/Polly.Core/Utils/PipelineComponent.Bridge.cs index f7fed2d91de..e0c3a50103d 100644 --- a/src/Polly.Core/Utils/PipelineComponent.Bridge.cs +++ b/src/Polly.Core/Utils/PipelineComponent.Bridge.cs @@ -3,9 +3,10 @@ internal abstract partial class PipelineComponent { [DebuggerDisplay("{Strategy}")] - internal sealed class BridgeComponent : PipelineComponent + internal sealed class BridgeComponent : BridgeComponentBase { - public BridgeComponent(ResilienceStrategy strategy) => Strategy = strategy; + public BridgeComponent(ResilienceStrategy strategy) + : base(strategy) => Strategy = strategy; public ResilienceStrategy Strategy { get; } @@ -38,9 +39,10 @@ static async (context, state) => } [DebuggerDisplay("{Strategy}")] - internal sealed class BridgeComponent : PipelineComponent + internal sealed class BridgeComponent : BridgeComponentBase { - public BridgeComponent(ResilienceStrategy strategy) => Strategy = strategy; + public BridgeComponent(ResilienceStrategy strategy) + : base(strategy) => Strategy = strategy; public ResilienceStrategy Strategy { get; } @@ -49,4 +51,37 @@ internal override ValueTask> ExecuteCore( ResilienceContext context, TState state) => Strategy.ExecuteCore(callback, context, state); } + + internal abstract class BridgeComponentBase : PipelineComponent + { + private readonly object _strategy; + + protected BridgeComponentBase(object strategy) => _strategy = strategy; + + public override void Dispose() + { + if (_strategy is IDisposable disposable) + { + disposable.Dispose(); + } + else if (_strategy is IAsyncDisposable asyncDisposable) + { + asyncDisposable.DisposeAsync().AsTask().GetAwaiter().GetResult(); + } + } + + public override ValueTask DisposeAsync() + { + if (_strategy is IAsyncDisposable asyncDisposable) + { + return asyncDisposable.DisposeAsync(); + } + else + { + Dispose(); + return default; + } + } + } + } diff --git a/src/Polly.Core/Utils/PipelineComponent.Composite.cs b/src/Polly.Core/Utils/PipelineComponent.Composite.cs index 1539595c1a5..a4401839bda 100644 --- a/src/Polly.Core/Utils/PipelineComponent.Composite.cs +++ b/src/Polly.Core/Utils/PipelineComponent.Composite.cs @@ -11,7 +11,6 @@ internal abstract partial class PipelineComponent [DebuggerTypeProxy(typeof(CompositeDebuggerProxy))] internal sealed class CompositeComponent : PipelineComponent { - private readonly PipelineComponent _firstComponent; private readonly ResilienceStrategyTelemetry _telemetry; private readonly TimeProvider _timeProvider; @@ -25,9 +24,11 @@ private CompositeComponent( _telemetry = telemetry; _timeProvider = timeProvider; - _firstComponent = first; + FirstComponent = first; } + internal PipelineComponent FirstComponent { get; } + public static PipelineComponent Create( IReadOnlyList components, ResilienceStrategyTelemetry telemetry, @@ -62,6 +63,22 @@ public static PipelineComponent Create( public IReadOnlyList Components { get; } + public override void Dispose() + { + foreach (var component in Components) + { + component.Dispose(); + } + } + + public override async ValueTask DisposeAsync() + { + foreach (var component in Components) + { + await component.DisposeAsync().ConfigureAwait(false); + } + } + internal override async ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, @@ -78,7 +95,7 @@ internal override async ValueTask> ExecuteCore } else { - outcome = await _firstComponent.ExecuteCore(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + outcome = await FirstComponent.ExecuteCore(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); } _telemetry.Report( @@ -118,6 +135,12 @@ internal override ValueTask> ExecuteCore( context, (Next, callback, state)); } + + public override void Dispose() + { + } + + public override ValueTask DisposeAsync() => default; } internal sealed class CompositeDebuggerProxy diff --git a/src/Polly.Core/Utils/PipelineComponent.Reloadale.cs b/src/Polly.Core/Utils/PipelineComponent.Reloadale.cs index e117f3938ee..96422482465 100644 --- a/src/Polly.Core/Utils/PipelineComponent.Reloadale.cs +++ b/src/Polly.Core/Utils/PipelineComponent.Reloadale.cs @@ -2,6 +2,8 @@ namespace Polly.Utils; +#pragma warning disable CA1031 // Do not catch general exception types + internal abstract partial class PipelineComponent { internal sealed class ReloadableComponent : PipelineComponent @@ -40,6 +42,18 @@ internal override ValueTask> ExecuteCore( return Component.ExecuteCore(callback, context, state); } + public override void Dispose() + { + DisposeRegistration(); + Component.Dispose(); + } + + public override ValueTask DisposeAsync() + { + DisposeRegistration(); + return Component.DisposeAsync(); + } + private void RegisterOnReload(CancellationToken previousToken) { var token = _onReload(); @@ -51,12 +65,14 @@ private void RegisterOnReload(CancellationToken previousToken) _registration = token.Register(() => { var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); + PipelineComponent previousComponent = Component; -#pragma warning disable CA1031 // Do not catch general exception types try { _telemetry.Report(new(ResilienceEventSeverity.Information, OnReloadEvent), context, new OnReloadArguments()); Component = _factory(); + + previousComponent.Dispose(); } catch (Exception e) { @@ -64,13 +80,16 @@ private void RegisterOnReload(CancellationToken previousToken) _telemetry.Report(new(ResilienceEventSeverity.Error, ReloadFailedEvent), args); ResilienceContextPool.Shared.Return(context); } -#pragma warning restore CA1031 // Do not catch general exception types - _registration.Dispose(); + DisposeRegistration(); RegisterOnReload(token); }); } +#pragma warning disable S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods + private void DisposeRegistration() => _registration.Dispose(); +#pragma warning restore S2952 // Classes should "Dispose" of members from the classes' own "Dispose" methods + internal readonly record struct ReloadFailedArguments(Exception Exception); internal readonly record struct OnReloadArguments(); diff --git a/src/Polly.Core/Utils/PipelineComponent.cs b/src/Polly.Core/Utils/PipelineComponent.cs index fa76c7f73d9..b9ae69bed75 100644 --- a/src/Polly.Core/Utils/PipelineComponent.cs +++ b/src/Polly.Core/Utils/PipelineComponent.cs @@ -11,7 +11,7 @@ namespace Polly.Utils; /// /// The component of the pipeline can be either a strategy, a generic strategy or a whole pipeline. /// -internal abstract partial class PipelineComponent +internal abstract partial class PipelineComponent : IDisposable, IAsyncDisposable { public static PipelineComponent Null { get; } = new NullComponent(); @@ -41,9 +41,19 @@ internal abstract ValueTask> ExecuteCore( ResilienceContext context, TState state); - internal class NullComponent : PipelineComponent + public abstract void Dispose(); + + public abstract ValueTask DisposeAsync(); + + private class NullComponent : PipelineComponent { internal override ValueTask> ExecuteCore(Func>> callback, ResilienceContext context, TState state) => callback(context, state); + + public override void Dispose() + { + } + + public override ValueTask DisposeAsync() => default; } } diff --git a/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs b/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs index 0508d2c9160..59274c95e98 100644 --- a/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs +++ b/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs @@ -3,7 +3,7 @@ namespace Polly.RateLimiting; -internal sealed class RateLimiterResilienceStrategy : ResilienceStrategy +internal sealed class RateLimiterResilienceStrategy : ResilienceStrategy, IDisposable, IAsyncDisposable { private readonly ResilienceStrategyTelemetry _telemetry; @@ -22,6 +22,10 @@ public RateLimiterResilienceStrategy( public Func? OnLeaseRejected { get; } + public void Dispose() => Limiter.Dispose(); + + public ValueTask DisposeAsync() => Limiter.DisposeAsync(); + protected override async ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerManualControlTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerManualControlTests.cs index 8af153f1369..c809dc5a56b 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerManualControlTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerManualControlTests.cs @@ -9,18 +9,17 @@ public class CircuitBreakerManualControlTests [Theory] public void Ctor_Isolated(bool isolated) { - using var control = new CircuitBreakerManualControl(isolated); + var control = new CircuitBreakerManualControl(isolated); var isolateCalled = false; - control.Initialize( + using var reg = control.Initialize( c => { c.IsSynchronous.Should().BeTrue(); isolateCalled = true; return Task.CompletedTask; }, - _ => Task.CompletedTask, - () => { }); + _ => Task.CompletedTask); isolateCalled.Should().Be(isolated); } @@ -30,7 +29,7 @@ public void Ctor_Isolated(bool isolated) [Theory] public async Task IsolateAsync_NotInitialized_Ok(bool closedAfter) { - using var control = new CircuitBreakerManualControl(); + var control = new CircuitBreakerManualControl(); await control.IsolateAsync(); if (closedAfter) { @@ -39,15 +38,14 @@ public async Task IsolateAsync_NotInitialized_Ok(bool closedAfter) var isolated = false; - control.Initialize( + using var reg = control.Initialize( c => { c.IsSynchronous.Should().BeTrue(); isolated = true; return Task.CompletedTask; }, - _ => Task.CompletedTask, - () => { }); + _ => Task.CompletedTask); isolated.Should().Be(!closedAfter); } @@ -55,7 +53,7 @@ public async Task IsolateAsync_NotInitialized_Ok(bool closedAfter) [Fact] public async Task ResetAsync_NotInitialized_Ok() { - using var control = new CircuitBreakerManualControl(); + var control = new CircuitBreakerManualControl(); await control .Invoking(c => c.CloseAsync(CancellationToken.None)) @@ -68,15 +66,31 @@ public async Task Initialize_Twice_Ok() { int called = 0; var control = new CircuitBreakerManualControl(); - control.Initialize(_ => Task.CompletedTask, _ => Task.CompletedTask, () => { }); - control.Initialize(_ => { called++; return Task.CompletedTask; }, _ => { called++; return Task.CompletedTask; }, () => { called++; }); + control.Initialize(_ => Task.CompletedTask, _ => Task.CompletedTask); + control.Initialize(_ => { called++; return Task.CompletedTask; }, _ => { called++; return Task.CompletedTask; }); await control.IsolateAsync(); await control.CloseAsync(); - control.Dispose(); + called.Should().Be(2); + } + + [Fact] + public async Task Initialize_DisposeRegistration_ShuldBeCancelled() + { + int called = 0; + var control = new CircuitBreakerManualControl(); + var reg = control.Initialize(_ => { called++; return Task.CompletedTask; }, _ => { called++; return Task.CompletedTask; }); - called.Should().Be(3); + await control.IsolateAsync(); + await control.CloseAsync(); + + reg.Dispose(); + + await control.IsolateAsync(); + await control.CloseAsync(); + + called.Should().Be(2); } [Fact] @@ -85,7 +99,6 @@ public async Task Initialize_Ok() var control = new CircuitBreakerManualControl(); var isolateCalled = false; var resetCalled = false; - var disposeCalled = false; control.Initialize( context => @@ -101,16 +114,12 @@ public async Task Initialize_Ok() context.IsSynchronous.Should().BeFalse(); resetCalled = true; return Task.CompletedTask; - }, - () => disposeCalled = true); + }); await control.IsolateAsync(CancellationToken.None); await control.CloseAsync(CancellationToken.None); - control.Dispose(); - isolateCalled.Should().BeTrue(); resetCalled.Should().BeTrue(); - disposeCalled.Should().BeTrue(); } } diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs index 94122e3162a..05f6eb62633 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResiliencePipelineBuilderTests.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Time.Testing; using Polly.CircuitBreaker; using Polly.Testing; +using Polly.Utils; namespace Polly.Core.Tests.CircuitBreaker; @@ -130,4 +131,43 @@ public async Task AddCircuitBreakers_WithIsolatedManualControl_ShouldBeIsolated( strategy1.Execute(() => { }); strategy2.Execute(() => { }); } + + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + [Theory] + public async Task DisposePipeline_EnsureCircuitBreakerDisposed(bool isAsync, bool attachManualControl) + { + var manualControl = attachManualControl ? new CircuitBreakerManualControl() : null; + var pipeline = new ResiliencePipelineBuilder() + .AddCircuitBreaker(new() + { + ManualControl = manualControl + }) + .Build(); + + if (attachManualControl) + { + manualControl!.IsEmpty.Should().BeFalse(); + } + + var strategy = (ResilienceStrategy)pipeline.GetPipelineDescriptor().FirstStrategy.StrategyInstance; + + if (isAsync) + { + await pipeline.DisposeHelper.DisposeAsync(); + } + else + { + pipeline.DisposeHelper.Dispose(); + } + + strategy.AsPipeline().Invoking(s => s.Execute(() => 1)).Should().Throw(); + + if (attachManualControl) + { + manualControl!.IsEmpty.Should().BeTrue(); + } + } } diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs index 8169de07377..2a17d58b477 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs @@ -61,9 +61,6 @@ public async Task Ctor_ManualControl_EnsureAttached() strategy.Invoking(s => s.Execute(_ => 0)).Should().NotThrow(); - _options.ManualControl.Dispose(); - strategy.Invoking(s => s.Execute(_ => 0)).Should().Throw(); - _behavior.Received().OnCircuitClosed(); _behavior.Received().OnActionSuccess(CircuitState.Closed); } @@ -135,5 +132,7 @@ public void Execute_Ok() } private ResiliencePipeline Create() +#pragma warning disable CA2000 // Dispose objects before losing scope => new CircuitBreakerResilienceStrategy(_options.ShouldHandle!, _controller, _options.StateProvider, _options.ManualControl).AsPipeline(); +#pragma warning restore CA2000 // Dispose objects before losing scope } diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerStateProviderTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerStateProviderTests.cs index 70d5a46ae5b..73e596a148f 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerStateProviderTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerStateProviderTests.cs @@ -24,7 +24,7 @@ public void NotInitialized_EnsureDefaults() [Fact] public async Task ResetAsync_NotInitialized_Throws() { - using var control = new CircuitBreakerManualControl(); + var control = new CircuitBreakerManualControl(); await control .Invoking(c => c.CloseAsync(CancellationToken.None)) diff --git a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs index f725f31dfaa..cd77197b644 100644 --- a/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs +++ b/test/Polly.Core.Tests/Registry/ResiliencePipelineRegistryTests.cs @@ -3,6 +3,7 @@ using Polly.Retry; using Polly.Testing; using Polly.Timeout; +using Polly.Utils; namespace Polly.Core.Tests.Registry; @@ -42,7 +43,7 @@ public void Ctor_InvalidOptions_Throws() public void GetPipeline_BuilderMultiInstance_EnsureMultipleInstances() { var builderName = "A"; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var strategies = new HashSet(); registry.TryAddBuilder(StrategyId.Create(builderName), (builder, _) => builder.AddStrategy(new TestResilienceStrategy())); @@ -63,7 +64,7 @@ public void GetPipeline_BuilderMultiInstance_EnsureMultipleInstances() public void GetPipeline_GenericBuilderMultiInstance_EnsureMultipleInstances() { var builderName = "A"; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var strategies = new HashSet>(); registry.TryAddBuilder(StrategyId.Create(builderName), (builder, _) => builder.AddStrategy(new TestResilienceStrategy())); @@ -85,7 +86,7 @@ public void TryAddBuilder_GetPipeline_EnsureCalled() { var activatorCalls = 0; _callback = _ => activatorCalls++; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var called = 0; registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => { @@ -113,7 +114,7 @@ public void TryAddBuilder_GenericGetPipeline_EnsureCalled() { var activatorCalls = 0; _callback = _ => activatorCalls++; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var called = 0; registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => { @@ -143,7 +144,7 @@ public void TryAddBuilder_EnsurePipelineKey() _options.InstanceNameFormatter = k => k.InstanceName; var called = false; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); registry.TryAddBuilder(StrategyId.Create("A"), (builder, context) => { context.BuilderName.Should().Be("A"); @@ -164,7 +165,7 @@ public void TryAddBuilder_EnsurePipelineKey() [Theory] public void TryAddBuilder_Twice_EnsureCorrectBehavior(bool generic) { - var registry = new ResiliencePipelineRegistry(); + using var registry = new ResiliencePipelineRegistry(); var called1 = false; var called2 = false; @@ -200,7 +201,7 @@ bool AddBuilder(Action onCalled) [Fact] public void TryAddBuilder_MultipleGeneric_EnsureDistinctInstances() { - var registry = CreateRegistry(); + using var registry = CreateRegistry(); registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => builder.AddStrategy(new TestResilienceStrategy())); registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => builder.AddStrategy(new TestResilienceStrategy())); @@ -215,7 +216,7 @@ public void TryAddBuilder_Generic_EnsurePipelineKey() _options.InstanceNameFormatter = k => k.InstanceName; var called = false; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); registry.TryAddBuilder(StrategyId.Create("A"), (builder, _) => { builder.AddStrategy(new TestResilienceStrategy()); @@ -231,7 +232,7 @@ public void TryAddBuilder_Generic_EnsurePipelineKey() [Fact] public void TryGet_NoBuilder_Null() { - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var key = StrategyId.Create("A"); registry.TryGetPipeline(key, out var strategy).Should().BeFalse(); @@ -241,7 +242,7 @@ public void TryGet_NoBuilder_Null() [Fact] public void TryGet_GenericNoBuilder_Null() { - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var key = StrategyId.Create("A"); registry.TryGetPipeline(key, out var strategy).Should().BeFalse(); @@ -253,7 +254,7 @@ public void EnableReloads_Ok() { // arrange var retryCount = 2; - var registry = new ResiliencePipelineRegistry(); + using var registry = new ResiliencePipelineRegistry(); using var changeSource = new CancellationTokenSource(); registry.TryAddBuilder("dummy", (builder, context) => @@ -289,7 +290,7 @@ public void EnableReloads_Generic_Ok() { // arrange var retryCount = 2; - var registry = new ResiliencePipelineRegistry(); + using var registry = new ResiliencePipelineRegistry(); using var changeSource = new CancellationTokenSource(); registry.TryAddBuilder("dummy", (builder, context) => @@ -326,7 +327,7 @@ public void GetOrAddPipeline_Ok() var id = new StrategyId(typeof(string), "A"); var called = 0; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var strategy = registry.GetOrAddPipeline(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); var otherPipeline = registry.GetOrAddPipeline(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); @@ -341,12 +342,81 @@ public void GetOrAddPipeline_Generic_Ok() var id = new StrategyId(typeof(string), "A"); var called = 0; - var registry = CreateRegistry(); + using var registry = CreateRegistry(); var strategy = registry.GetOrAddPipeline(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); var otherPipeline = registry.GetOrAddPipeline(id, builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); called++; }); strategy.GetPipelineDescriptor().FirstStrategy.StrategyInstance.Should().BeOfType(); } + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task Dispose_EnsureDisposed(bool isAsync) + { + var registry = CreateRegistry(); + + var pipeline1 = registry.GetOrAddPipeline(StrategyId.Create("A"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); + var pipeline2 = registry.GetOrAddPipeline(StrategyId.Create("B"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); + var pipeline3 = registry.GetOrAddPipeline(StrategyId.Create("C"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); + var pipeline4 = registry.GetOrAddPipeline(StrategyId.Create("D"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); + +#pragma warning disable S3966 // Objects should not be disposed more than once + if (isAsync) + { + await registry.DisposeAsync(); + await registry.DisposeAsync(); + } + else + { + registry.Dispose(); + registry.Dispose(); + } +#pragma warning restore S3966 // Objects should not be disposed more than once + + pipeline1.Invoking(p => p.Execute(() => { })).Should().Throw(); + pipeline2.Invoking(p => p.Execute(() => { })).Should().Throw(); + pipeline3.Invoking(p => p.Execute(() => "dummy")).Should().Throw(); + pipeline4.Invoking(p => p.Execute(() => "dummy")).Should().Throw(); + } + + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task DisposePipeline_NotAllowed(bool isAsync) + { + using var registry = CreateRegistry(); + var pipeline = registry.GetOrAddPipeline(StrategyId.Create("A"), builder => { builder.AddTimeout(TimeSpan.FromSeconds(1)); }); + + if (isAsync) + { + await pipeline.Invoking(p => p.DisposeHelper.DisposeAsync().AsTask()).Should().ThrowAsync(); + } + else + { + pipeline.Invoking(p => p.DisposeHelper.Dispose()).Should().Throw(); + } + } + + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task Dispose_EnsureNotUsableAnymore(bool isAsync) + { + using var registry = new ResiliencePipelineRegistry(); + await DisposeHelper.TryDisposeSafeAsync(registry, !isAsync); + + registry.Invoking(r => r.GetOrAddPipeline("dummy", builder => { })).Should().Throw(); + registry.Invoking(r => r.GetOrAddPipeline("dummy", builder => { })).Should().Throw(); + registry.Invoking(r => r.GetOrAddPipeline("dummy", (_, _) => { })).Should().Throw(); + registry.Invoking(r => r.GetOrAddPipeline("dummy", (_, _) => { })).Should().Throw(); + registry.Invoking(r => r.TryAddBuilder("dummy", (_, _) => { })).Should().Throw(); + registry.Invoking(r => r.TryAddBuilder("dummy", (_, _) => { })).Should().Throw(); + registry.Invoking(r => r.GetPipeline("dummy")).Should().Throw(); + registry.Invoking(r => r.GetPipeline("dummy")).Should().Throw(); + registry.Invoking(r => r.TryGetPipeline("dummy", out _)).Should().Throw(); + registry.Invoking(r => r.TryGetPipeline("dummy", out _)).Should().Throw(); + } + private ResiliencePipelineRegistry CreateRegistry() => new(_options); } diff --git a/test/Polly.Core.Tests/ResiliencePipelineTTests.Async.cs b/test/Polly.Core.Tests/ResiliencePipelineTTests.Async.cs index 78ee73b24fe..da7d9c9e638 100644 --- a/test/Polly.Core.Tests/ResiliencePipelineTTests.Async.cs +++ b/test/Polly.Core.Tests/ResiliencePipelineTTests.Async.cs @@ -73,7 +73,7 @@ public async Task ExecuteAsync_GenericStrategy_Ok(Func> execut c.IsSynchronous.Should().BeTrue(); c.ResultType.Should().Be(typeof(string)); }, - })); + }), DisposeBehavior.Allow); execute(pipeline); } diff --git a/test/Polly.Core.Tests/ResiliencePipelineTests.cs b/test/Polly.Core.Tests/ResiliencePipelineTests.cs index ad463e0b907..0fda45e2b05 100644 --- a/test/Polly.Core.Tests/ResiliencePipelineTests.cs +++ b/test/Polly.Core.Tests/ResiliencePipelineTests.cs @@ -3,10 +3,77 @@ namespace Polly.Core.Tests; +#pragma warning disable S3966 // Objects should not be disposed more than once + public partial class ResiliencePipelineTests { public static readonly CancellationToken CancellationToken = new CancellationTokenSource().Token; + [Fact] + public async Task Dispose_NullPipeline_OK() + { + ResiliencePipeline.Null.DisposeHelper.Dispose(); + ResiliencePipeline.Null.DisposeHelper.Dispose(); + await ResiliencePipeline.Null.DisposeHelper.DisposeAsync(); + await ResiliencePipeline.Null.DisposeHelper.DisposeAsync(); + + ResiliencePipeline.Null.Execute(() => 1).Should().Be(1); + } + + [Fact] + public async Task Dispose_NullGenericPipeline_OK() + { + ResiliencePipeline.Null.DisposeHelper.Dispose(); + ResiliencePipeline.Null.DisposeHelper.Dispose(); + await ResiliencePipeline.Null.DisposeHelper.DisposeAsync(); + await ResiliencePipeline.Null.DisposeHelper.DisposeAsync(); + + ResiliencePipeline.Null.Execute(() => 1).Should().Be(1); + } + + [Fact] + public async Task Dispose_Reject_Throws() + { + var component = Substitute.For(); + var pipeline = new ResiliencePipeline(component, DisposeBehavior.Reject); + + pipeline.Invoking(p => p.DisposeHelper.Dispose()) + .Should() + .Throw() + .WithMessage("Disposing this resilience pipeline is not allowed because it is owned by the pipeline registry."); + + (await pipeline.Invoking(p => p.DisposeHelper.DisposeAsync().AsTask()) + .Should() + .ThrowAsync()) + .WithMessage("Disposing this resilience pipeline is not allowed because it is owned by the pipeline registry."); + } + + [Fact] + public void Dispose_Allowed_Disposed() + { + var component = Substitute.For(); + var pipeline = new ResiliencePipeline(component, DisposeBehavior.Allow); + pipeline.DisposeHelper.Dispose(); + pipeline.DisposeHelper.Dispose(); + + pipeline.Invoking(p => p.Execute(() => { })).Should().Throw(); + + component.Received(1).Dispose(); + } + + [Fact] + public async Task DisposeAsync_Allowed_Disposed() + { + var component = Substitute.For(); + var pipeline = new ResiliencePipeline(component, DisposeBehavior.Allow); + await pipeline.DisposeHelper.DisposeAsync(); + await pipeline.DisposeHelper.DisposeAsync(); + + pipeline.Invoking(p => p.Execute(() => { })).Should().Throw(); + + await component.Received(1).DisposeAsync(); + } + [Fact] public void Null_Ok() { @@ -17,7 +84,7 @@ public void Null_Ok() [Fact] public void DebuggerProxy_Ok() { - var pipeline = (PipelineComponent.CompositeComponent)PipelineComponent.CreateComposite(new[] + using var pipeline = (PipelineComponent.CompositeComponent)PipelineComponent.CreateComposite(new[] { Substitute.For(), Substitute.For(), diff --git a/test/Polly.Core.Tests/Utils/PipelineComponents/BridgePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/PipelineComponents/BridgePipelineComponentTests.cs index bf128ba2bd2..e4f3e73fb41 100644 --- a/test/Polly.Core.Tests/Utils/PipelineComponents/BridgePipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/PipelineComponents/BridgePipelineComponentTests.cs @@ -1,4 +1,5 @@ -using Polly.Utils; +using NSubstitute; +using Polly.Utils; namespace Polly.Core.Tests.Utils.PipelineComponents; @@ -18,7 +19,7 @@ public void Execute_NonGeneric_Ok() var pipeline = new ResiliencePipeline(PipelineComponent.FromStrategy(new Strategy(outcome => { values.Add(outcome.Result); - }))); + })), DisposeBehavior.Allow); pipeline.Execute(args => "dummy"); pipeline.Execute(args => 0); @@ -39,7 +40,7 @@ public void Execute_Generic_Ok() var pipeline = new ResiliencePipeline(PipelineComponent.FromStrategy(new Strategy(outcome => { values.Add(outcome.Result); - }))); + })), DisposeBehavior.Allow); pipeline.Execute(args => "dummy"); @@ -56,13 +57,55 @@ public void Pipeline_TypeCheck_Ok() { outcome.Result.Should().Be(-1); called = true; - }))); + })), DisposeBehavior.Allow); pipeline.Execute(() => -1); called.Should().BeTrue(); } +#pragma warning disable S1944 // Invalid casts should be avoided + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task Dispose_EnsureStrategyDisposed(bool isAsync) + { + var strategy = Substitute.For(); + await Dispose(PipelineComponent.FromStrategy(strategy), isAsync); + ((IDisposable)strategy).Received(1).Dispose(); + + strategy = Substitute.For(); + await Dispose(PipelineComponent.FromStrategy(strategy), isAsync); + await ((IAsyncDisposable)strategy).Received(1).DisposeAsync(); + } + + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task Dispose_Generic_EnsureStrategyDisposed(bool isAsync) + { + var strategy = Substitute.For, IDisposable>(); + await Dispose(PipelineComponent.FromStrategy(strategy), isAsync); + ((IDisposable)strategy).Received(1).Dispose(); + + strategy = Substitute.For, IAsyncDisposable>(); + await Dispose(PipelineComponent.FromStrategy(strategy), isAsync); + await ((IAsyncDisposable)strategy).Received(1).DisposeAsync(); + } +#pragma warning restore S1944 // Invalid casts should be avoided + + private static async Task Dispose(PipelineComponent component, bool isAsync) + { + if (isAsync) + { + await component.DisposeAsync(); + } + else + { + component.Dispose(); + } + } + private class Strategy : ResilienceStrategy { private readonly Action> _onOutcome; diff --git a/test/Polly.Core.Tests/Utils/PipelineComponents/CompositePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/PipelineComponents/CompositePipelineComponentTests.cs index 367a1a47ed6..4121854dbbe 100644 --- a/test/Polly.Core.Tests/Utils/PipelineComponents/CompositePipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/PipelineComponents/CompositePipelineComponentTests.cs @@ -5,6 +5,8 @@ namespace Polly.Core.Tests.Utils.PipelineComponents; +#pragma warning disable CA2000 // Dispose objects before losing scope + public class CompositePipelineComponentTests { private readonly ResilienceStrategyTelemetry _telemetry; @@ -83,7 +85,7 @@ public async Task Create_Cancelled_EnsureNoExecution() PipelineComponent.FromStrategy(new TestResilienceStrategy()), }; - var pipeline = new ResiliencePipeline(CreateSut(strategies, new FakeTimeProvider())); + var pipeline = new ResiliencePipeline(CreateSut(strategies, new FakeTimeProvider()), DisposeBehavior.Allow); var context = ResilienceContextPool.Shared.Get(); context.CancellationToken = cancellation.Token; @@ -98,10 +100,10 @@ public async Task Create_CancelledLater_EnsureNoExecution() using var cancellation = new CancellationTokenSource(); var strategies = new[] { - PipelineComponent.FromStrategy( new TestResilienceStrategy { Before = (_, _) => { executed = true; cancellation.Cancel(); } }), + PipelineComponent.FromStrategy(new TestResilienceStrategy { Before = (_, _) => { executed = true; cancellation.Cancel(); } }), PipelineComponent.FromStrategy(new TestResilienceStrategy()), }; - var pipeline = new ResiliencePipeline(CreateSut(strategies, new FakeTimeProvider())); + var pipeline = new ResiliencePipeline(CreateSut(strategies, new FakeTimeProvider()), DisposeBehavior.Allow); var context = ResilienceContextPool.Shared.Get(); context.CancellationToken = cancellation.Token; @@ -115,7 +117,7 @@ public void ExecutePipeline_EnsureTelemetryArgumentsReported() { var timeProvider = new FakeTimeProvider(); - var pipeline = new ResiliencePipeline(CreateSut(new[] { Substitute.For() }, timeProvider)); + var pipeline = new ResiliencePipeline(CreateSut(new[] { Substitute.For() }, timeProvider), DisposeBehavior.Allow); pipeline.Execute(() => { timeProvider.Advance(TimeSpan.FromHours(1)); }); _listener.Events.Should().HaveCount(2); @@ -123,6 +125,35 @@ public void ExecutePipeline_EnsureTelemetryArgumentsReported() _listener.GetArgs().Should().HaveCount(1); } + [Fact] + public void Dispose_EnsureInnerComponentsDisposed() + { + var a = Substitute.For(); + var b = Substitute.For(); + + var composite = CreateSut(new[] { a, b }); + + composite.FirstComponent.Dispose(); + composite.Dispose(); + + a.Received(1).Dispose(); + b.Received(1).Dispose(); + } + + [Fact] + public async Task DisposeAsync_EnsureInnerComponentsDisposed() + { + var a = Substitute.For(); + var b = Substitute.For(); + + var composite = CreateSut(new[] { a, b }); + await composite.FirstComponent.DisposeAsync(); + await composite.DisposeAsync(); + + await a.Received(1).DisposeAsync(); + await b.Received(1).DisposeAsync(); + } + private PipelineComponent.CompositeComponent CreateSut(PipelineComponent[] components, TimeProvider? timeProvider = null) { return (PipelineComponent.CompositeComponent)PipelineComponent.CreateComposite(components, _telemetry, timeProvider ?? Substitute.For()); diff --git a/test/Polly.Core.Tests/Utils/PipelineComponents/PipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/PipelineComponents/PipelineComponentTests.cs index 829f185422b..f90c065f744 100644 --- a/test/Polly.Core.Tests/Utils/PipelineComponents/PipelineComponentTests.cs +++ b/test/Polly.Core.Tests/Utils/PipelineComponents/PipelineComponentTests.cs @@ -1,91 +1,15 @@ -using NSubstitute; -using Polly.Telemetry; +using System.Threading.Tasks; using Polly.Utils; namespace Polly.Core.Tests.Utils.PipelineComponents; -public class PipelineComponentTests : IDisposable +public class PipelineComponentTests { - private readonly List> _events = new(); - private readonly ResilienceStrategyTelemetry _telemetry; - private CancellationTokenSource _cancellationTokenSource; - - public PipelineComponentTests() - { - _telemetry = TestUtilities.CreateResilienceTelemetry(args => - { - args.Context.IsSynchronous.Should().BeTrue(); - args.Context.IsVoid.Should().BeTrue(); - _events.Add(args); - }); - - _cancellationTokenSource = new CancellationTokenSource(); - } - - [Fact] - public void Ctor_Ok() - { - var component = Substitute.For(); - var sut = CreateSut(component); - - sut.Component.Should().Be(component); - - PipelineComponent.ReloadableComponent.ReloadFailedEvent.Should().Be("ReloadFailed"); - } - - [Fact] - public void ChangeTriggered_StrategyReloaded() - { - var component = Substitute.For(); - var sut = CreateSut(component); - - for (var i = 0; i < 10; i++) - { - var src = _cancellationTokenSource; - _cancellationTokenSource = new CancellationTokenSource(); - src.Cancel(); - - sut.Component.Should().NotBe(component); - } - - _events.Where(e => e.Event.EventName == "ReloadFailed").Should().HaveCount(0); - _events.Where(e => e.Event.EventName == "OnReload").Should().HaveCount(10); - } - [Fact] - public void ChangeTriggered_FactoryError_LastStrategyUsedAndErrorReported() - { - var component = Substitute.For(); - var sut = CreateSut(component, () => throw new InvalidOperationException()); - - _cancellationTokenSource.Cancel(); - - sut.Component.Should().Be(component); - _events.Should().HaveCount(2); - - _events[0] - .Arguments - .Should() - .BeOfType(); - - var args = _events[1] - .Arguments - .Should() - .BeOfType() - .Subject; - - args.Exception.Should().BeOfType(); - } - - private PipelineComponent.ReloadableComponent CreateSut(PipelineComponent? initial = null, Func? factory = null) + public async Task Dispose_Ok() { - factory ??= () => PipelineComponent.Null; - - return (PipelineComponent.ReloadableComponent)PipelineComponent.CreateReloadable(initial ?? PipelineComponent.Null, - () => _cancellationTokenSource.Token, - factory, - _telemetry); + PipelineComponent.Null.Should().NotBeNull(); + PipelineComponent.Null.Dispose(); + await PipelineComponent.Null.DisposeAsync(); } - - public void Dispose() => _cancellationTokenSource.Dispose(); } diff --git a/test/Polly.Core.Tests/Utils/PipelineComponents/ReloadablePipelineComponentTests.cs b/test/Polly.Core.Tests/Utils/PipelineComponents/ReloadablePipelineComponentTests.cs new file mode 100644 index 00000000000..ebf5771e302 --- /dev/null +++ b/test/Polly.Core.Tests/Utils/PipelineComponents/ReloadablePipelineComponentTests.cs @@ -0,0 +1,123 @@ +using NSubstitute; +using Polly.Telemetry; +using Polly.Utils; + +namespace Polly.Core.Tests.Utils.PipelineComponents; + +public class ReloadablePipelineComponentTests : IDisposable +{ + private readonly List> _events = new(); + private readonly ResilienceStrategyTelemetry _telemetry; + private CancellationTokenSource _cancellationTokenSource; + + public ReloadablePipelineComponentTests() + { + _telemetry = TestUtilities.CreateResilienceTelemetry(args => + { + args.Context.IsSynchronous.Should().BeTrue(); + args.Context.IsVoid.Should().BeTrue(); + _events.Add(args); + }); + + _cancellationTokenSource = new CancellationTokenSource(); + } + + [Fact] + public void Ctor_Ok() + { + var component = Substitute.For(); + using var sut = CreateSut(component); + + sut.Component.Should().Be(component); + + PipelineComponent.ReloadableComponent.ReloadFailedEvent.Should().Be("ReloadFailed"); + } + + [Fact] + public void Dispose_ComponentDisposed() + { + var component = Substitute.For(); + CreateSut(component).Dispose(); + component.Received(1).Dispose(); + } + + [Fact] + public async Task DisposeAsync_ComponentDisposed() + { + var component = Substitute.For(); + await CreateSut(component).DisposeAsync(); + await component.Received(1).DisposeAsync(); + } + + [Fact] + public void ChangeTriggered_StrategyReloaded() + { + var component = Substitute.For(); + using var sut = CreateSut(component); + + for (var i = 0; i < 10; i++) + { + var src = _cancellationTokenSource; + _cancellationTokenSource = new CancellationTokenSource(); + src.Cancel(); + + sut.Component.Should().NotBe(component); + } + + _events.Where(e => e.Event.EventName == "ReloadFailed").Should().HaveCount(0); + _events.Where(e => e.Event.EventName == "OnReload").Should().HaveCount(10); + } + + [Fact] + public void ChangeTriggered_EnsureOldStrategyDisposed() + { + var component = Substitute.For(); + using var sut = CreateSut(component, () => Substitute.For()); + + for (var i = 0; i < 10; i++) + { + var src = _cancellationTokenSource; + _cancellationTokenSource = new CancellationTokenSource(); + src.Cancel(); + component.Received(1).Dispose(); + sut.Component.Received(0).Dispose(); + } + } + + [Fact] + public void ChangeTriggered_FactoryError_LastStrategyUsedAndErrorReported() + { + var component = Substitute.For(); + using var sut = CreateSut(component, () => throw new InvalidOperationException()); + + _cancellationTokenSource.Cancel(); + + sut.Component.Should().Be(component); + _events.Should().HaveCount(2); + + _events[0] + .Arguments + .Should() + .BeOfType(); + + var args = _events[1] + .Arguments + .Should() + .BeOfType() + .Subject; + + args.Exception.Should().BeOfType(); + } + + private PipelineComponent.ReloadableComponent CreateSut(PipelineComponent? initial = null, Func? factory = null) + { + factory ??= () => PipelineComponent.Null; + + return (PipelineComponent.ReloadableComponent)PipelineComponent.CreateReloadable(initial ?? PipelineComponent.Null, + () => _cancellationTokenSource.Token, + factory, + _telemetry); + } + + public void Dispose() => _cancellationTokenSource.Dispose(); +} diff --git a/test/Polly.Extensions.Tests/ReloadableResiliencePipelineTests.cs b/test/Polly.Extensions.Tests/ReloadableResiliencePipelineTests.cs index 662b41fb2f1..d3b71721fdc 100644 --- a/test/Polly.Extensions.Tests/ReloadableResiliencePipelineTests.cs +++ b/test/Polly.Extensions.Tests/ReloadableResiliencePipelineTests.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using NSubstitute; using Polly.DependencyInjection; using Polly.Registry; @@ -15,6 +16,7 @@ public class ReloadableResiliencePipelineTests [Theory] public void AddResiliencePipeline_EnsureReloadable(string? name) { + var resList = new List(); var reloadableConfig = new ReloadableConfiguration(); reloadableConfig.Reload(new() { { "tag", "initial-tag" } }); var builder = new ConfigurationBuilder().Add(reloadableConfig); @@ -35,7 +37,13 @@ public void AddResiliencePipeline_EnsureReloadable(string? name) var options = context.GetOptions(name); context.EnableReloads(name); - builder.AddStrategy(_ => new ReloadableStrategy(options.Tag), new ReloadableStrategyOptions()); + builder.AddStrategy(_ => + { + var res = Substitute.For(); + resList.Add(res); + return new ReloadableStrategy(options.Tag, res); + }, + new ReloadableStrategyOptions()); }); var serviceProvider = services.BuildServiceProvider(); @@ -53,14 +61,37 @@ public void AddResiliencePipeline_EnsureReloadable(string? name) pipeline.Execute(_ => "dummy", context); context.Properties.GetValue(TagKey, string.Empty).Should().Be($"reload-{i}"); } + + // check resource disposed + resList.Should().HaveCount(11); + for (int i = 0; i < resList.Count - 1; i++) + { + resList[i].Received(1).Dispose(); + } + + resList.Last().Received(0).Dispose(); + + // check disposal of service provider + serviceProvider.Dispose(); + resList.Last().Received(1).Dispose(); + pipeline.Invoking(p => p.Execute(() => { })).Should().Throw(); + } - public class ReloadableStrategy : ResilienceStrategy + public class ReloadableStrategy : ResilienceStrategy, IDisposable { - public ReloadableStrategy(string tag) => Tag = tag; + public ReloadableStrategy(string tag, IDisposable disposableResource) + { + Tag = tag; + DisposableResource = disposableResource; + } public string Tag { get; } + public IDisposable DisposableResource { get; } + + public void Dispose() => DisposableResource.Dispose(); + protected override ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, diff --git a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs index 2b067059719..e98fd71e6bc 100644 --- a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs +++ b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs @@ -1,7 +1,9 @@ using System.ComponentModel.DataAnnotations; using System.Threading.RateLimiting; using NSubstitute; +using Polly.Registry; using Polly.Testing; +using Polly.TestUtils; namespace Polly.RateLimiting.Tests; @@ -137,6 +139,28 @@ public void AddRateLimiter_Options_Ok() .BeOfType(); } + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task DisposeRegistry_EnsureRateLimiterDisposed(bool isAsync) + { + var registry = new ResiliencePipelineRegistry(); + var pipeline = registry.GetOrAddPipeline("limiter", p => p.AddRateLimiter(new RateLimiterStrategyOptions())); + + var strategy = (RateLimiterResilienceStrategy)pipeline.GetPipelineDescriptor().FirstStrategy.StrategyInstance; + + if (isAsync) + { + await registry.DisposeAsync(); + } + else + { + registry.Dispose(); + } + + strategy.AsPipeline().Invoking(p => p.Execute(() => { })).Should().Throw(); + } + private static void AssertRateLimiterStrategy(ResiliencePipelineBuilder builder, Action? assert = null, bool hasEvents = false) { ResiliencePipeline strategy = builder.Build(); diff --git a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs index 681c63fdcd9..b27568ae4c8 100644 --- a/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs +++ b/test/Polly.Testing.Tests/ResiliencePipelineExtensionsTests.cs @@ -112,7 +112,8 @@ public void GetPipelineDescriptor_SingleStrategy_Ok() public void GetPipelineDescriptor_Reloadable_Ok() { // arrange - var strategy = new ResiliencePipelineRegistry().GetOrAddPipeline("dummy", (builder, context) => + using var registry = new ResiliencePipelineRegistry(); + var strategy = registry.GetOrAddPipeline("dummy", (builder, context) => { context.EnableReloads(() => () => CancellationToken.None);