From cd7a6bfc7e85e887e7437813685fed610b9df679 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Fri, 4 Jul 2025 13:18:11 +0300 Subject: [PATCH 01/10] Reduce async overhead --- .../CircuitBreakerResilienceStrategy.cs | 16 +++- .../Controller/CircuitStateController.cs | 74 +++++++++-------- .../Controller/ScheduledTaskExecutor.cs | 27 +++---- src/Polly.Core/Fallback/FallbackHandler.cs | 6 +- .../Fallback/FallbackResilienceStrategy.cs | 13 ++- .../Controller/HedgingExecutionContext.cs | 48 +++++------ .../Hedging/Controller/HedgingHandler.cs | 12 +-- .../Hedging/Controller/TaskExecution.cs | 69 ++++++++-------- .../Hedging/HedgingResilienceStrategy.cs | 81 ++++++++----------- .../Hedging/HedgingStrategyOptions.TResult.cs | 10 ++- .../RegistryPipelineComponentBuilder.cs | 36 +++------ .../ResiliencePipelineRegistry.TResult.cs | 30 ++++--- .../Registry/ResiliencePipelineRegistry.cs | 53 +++++------- .../Retry/RetryResilienceStrategy.cs | 13 ++- .../Retry/RetryStrategyOptions.TResult.cs | 2 +- .../Simmy/Behavior/ChaosBehaviorStrategy.cs | 12 ++- src/Polly.Core/Simmy/ChaosStrategy.cs | 6 +- src/Polly.Core/Simmy/ChaosStrategyOptions.cs | 2 +- .../Simmy/Fault/ChaosFaultStrategy.cs | 12 ++- src/Polly.Core/Simmy/Fault/FaultGenerator.cs | 3 +- .../Simmy/Latency/ChaosLatencyStrategy.cs | 31 ++++--- .../Simmy/Outcomes/ChaosOutcomeStrategy.cs | 12 ++- .../Simmy/Outcomes/OutcomeGenerator.cs | 8 +- .../Timeout/TimeoutResilienceStrategy.cs | 13 ++- .../Utils/Pipeline/BridgeComponent.TResult.cs | 48 +++-------- .../Utils/Pipeline/BridgeComponentBase.cs | 10 +++ .../Pipeline/PipelineComponentFactory.cs | 24 +----- src/Polly.Core/Utils/RandomUtil.cs | 20 +++-- src/Polly.Core/Utils/StrategyHelper.cs | 46 ----------- src/Polly.RateLimiting/DisposeWrapper.cs | 14 ---- ...iterResiliencePipelineBuilderExtensions.cs | 4 +- .../RateLimiterResilienceStrategy.cs | 4 +- .../Controller/CircuitStateControllerTests.cs | 2 +- .../Controller/ScheduledTaskExecutorTests.cs | 52 ++++-------- .../Fallback/FallbackHandlerTests.cs | 2 +- .../Hedging/HedgingHandlerTests.cs | 33 -------- .../Retry/RetryHelperTests.cs | 12 +-- .../Pipeline/PipelineComponentFactoryTests.cs | 4 +- .../Polly.Core.Tests/Utils/RandomUtilTests.cs | 15 +--- .../Utils/StrategyHelperTests.cs | 65 --------------- ...esiliencePipelineBuilderExtensionsTests.cs | 4 +- .../RateLimiterResilienceStrategyTests.cs | 3 +- 42 files changed, 367 insertions(+), 584 deletions(-) delete mode 100644 src/Polly.Core/Utils/StrategyHelper.cs delete mode 100644 src/Polly.RateLimiting/DisposeWrapper.cs delete mode 100644 test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs delete mode 100644 test/Polly.Core.Tests/Utils/StrategyHelperTests.cs diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index e6d8bd1927e..2eeb24f5699 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -17,8 +17,8 @@ public CircuitBreakerResilienceStrategy( stateProvider?.Initialize(() => _controller.CircuitState); _manualControlRegistration = manualControl?.Initialize( - async c => await _controller.IsolateCircuitAsync(c).ConfigureAwait(c.ContinueOnCapturedContext), - async c => await _controller.CloseCircuitAsync(c).ConfigureAwait(c.ContinueOnCapturedContext)); + _controller.IsolateCircuitAsync, + _controller.CloseCircuitAsync); } public void Dispose() @@ -34,7 +34,17 @@ protected internal override async ValueTask> ExecuteCore(Func return outcome; } - outcome = await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + outcome = new(ex); + } +#pragma warning restore CA1031 var args = new CircuitBreakerPredicateArguments(context, outcome); if (await _handler(args).ConfigureAwait(context.ContinueOnCapturedContext)) diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs index 53b4a3c04ed..3173c64b7f8 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs @@ -85,7 +85,7 @@ public Outcome? LastHandledOutcome } } - public ValueTask IsolateCircuitAsync(ResilienceContext context) + public Task IsolateCircuitAsync(ResilienceContext context) { EnsureNotDisposed(); @@ -98,14 +98,14 @@ public ValueTask IsolateCircuitAsync(ResilienceContext context) var exception = new IsolatedCircuitException(); _telemetry.SetTelemetrySource(exception); SetLastHandledOutcome_NeedsLock(Outcome.FromException(exception)); - OpenCircuitFor_NeedsLock(Outcome.FromResult(default), TimeSpan.MaxValue, manual: true, context, out task); + task = OpenCircuitFor_NeedsLock(Outcome.FromResult(default), TimeSpan.MaxValue, manual: true, context); _circuitState = CircuitState.Isolated; } return ExecuteScheduledTaskAsync(task, context); } - public ValueTask CloseCircuitAsync(ResilienceContext context) + public Task CloseCircuitAsync(ResilienceContext context) { EnsureNotDisposed(); @@ -115,7 +115,7 @@ public ValueTask CloseCircuitAsync(ResilienceContext context) lock (_lock) { - CloseCircuit_NeedsLock(Outcome.FromResult(default), manual: true, context, out task); + task = CloseCircuit_NeedsLock(Outcome.FromResult(default), manual: true, context); } return ExecuteScheduledTaskAsync(task, context); @@ -166,7 +166,7 @@ public ValueTask CloseCircuitAsync(ResilienceContext context) return null; } - public ValueTask OnUnhandledOutcomeAsync(Outcome outcome, ResilienceContext context) + public Task OnUnhandledOutcomeAsync(Outcome outcome, ResilienceContext context) { EnsureNotDisposed(); @@ -184,7 +184,7 @@ public ValueTask OnUnhandledOutcomeAsync(Outcome outcome, ResilienceContext c // We take no special action; only time passing governs transitioning from Open to HalfOpen state. if (_circuitState == CircuitState.HalfOpen) { - CloseCircuit_NeedsLock(outcome, manual: false, context, out task); + task = CloseCircuit_NeedsLock(outcome, manual: false, context); } } @@ -192,7 +192,7 @@ public ValueTask OnUnhandledOutcomeAsync(Outcome outcome, ResilienceContext c return ExecuteScheduledTaskAsync(task, context); } - public ValueTask OnHandledOutcomeAsync(Outcome outcome, ResilienceContext context) + public Task OnHandledOutcomeAsync(Outcome outcome, ResilienceContext context) { EnsureNotDisposed(); @@ -214,7 +214,7 @@ public ValueTask OnHandledOutcomeAsync(Outcome outcome, ResilienceContext con if (_circuitState == CircuitState.HalfOpen || (_circuitState == CircuitState.Closed && shouldBreak)) { - OpenCircuit_NeedsLock(outcome, manual: false, context, out task); + task = OpenCircuitFor_NeedsLock(outcome, _breakDuration, manual: false, context); } } @@ -227,22 +227,35 @@ public void Dispose() _disposed = true; } - internal static async ValueTask ExecuteScheduledTaskAsync(Task? task, ResilienceContext context) + internal static Task ExecuteScheduledTaskAsync(Task? task, ResilienceContext context) { if (task is not null) { - if (context.IsSynchronous) + if (context.IsSynchronous && !task.IsCompleted) { #pragma warning disable CA1849 // Call async methods when in an async method // because this is synchronous execution we need to block - task.GetAwaiter().GetResult(); +#if NET8_0_OR_GREATER + task.ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing).GetAwaiter().GetResult(); +#else + try + { + task.GetAwaiter().GetResult(); + } +#pragma warning disable CA1031 + catch + { + // exception will be observed by the awaiter of this method + } +#pragma warning restore CA1031 +#endif #pragma warning restore CA1849 // Call async methods when in an async method } - else - { - await task.ConfigureAwait(context.ContinueOnCapturedContext); - } + + return task; } + + return Task.CompletedTask; } private static bool IsDateTimeOverflow(DateTimeOffset utcNow, TimeSpan breakDuration) @@ -266,10 +279,8 @@ private void EnsureNotDisposed() } #endif - private void CloseCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceContext context, out Task? scheduledTask) + private Task? CloseCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceContext context) { - scheduledTask = null; - _blockedUntil = DateTimeOffset.MinValue; _lastOutcome = null; _halfOpenAttempts = 0; @@ -285,9 +296,13 @@ private void CloseCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceC if (_onClosed is not null) { - _executor.ScheduleTask(() => _onClosed(args).AsTask(), context, out scheduledTask); + return _executor.ScheduleTask(() => _onClosed(args).AsTask()); } } + +#pragma warning disable S4586 + return null; +#pragma warning restore S4586 } private bool PermitHalfOpenCircuitTest_NeedsLock() @@ -311,21 +326,13 @@ private void SetLastHandledOutcome_NeedsLock(Outcome outcome) private BrokenCircuitException CreateBrokenCircuitException() { TimeSpan retryAfter = _blockedUntil - _timeProvider.GetUtcNow(); - var exception = _breakingException switch - { - Exception ex => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter, ex), - _ => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter) - }; + var exception = new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter, _breakingException!); _telemetry.SetTelemetrySource(exception); return exception; } - private void OpenCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceContext context, out Task? scheduledTask) - => OpenCircuitFor_NeedsLock(outcome, _breakDuration, manual, context, out scheduledTask); - - private void OpenCircuitFor_NeedsLock(Outcome outcome, TimeSpan breakDuration, bool manual, ResilienceContext context, out Task? scheduledTask) + private Task? OpenCircuitFor_NeedsLock(Outcome outcome, TimeSpan breakDuration, bool manual, ResilienceContext context) { - scheduledTask = null; var utcNow = _timeProvider.GetUtcNow(); if (_breakDurationGenerator is not null) @@ -345,14 +352,17 @@ private void OpenCircuitFor_NeedsLock(Outcome outcome, TimeSpan breakDuration if (_onOpened is not null) { - _executor.ScheduleTask(() => _onOpened(args).AsTask(), context, out scheduledTask); + return _executor.ScheduleTask(() => _onOpened(args).AsTask()); } + +#pragma warning disable S4586 + return null; +#pragma warning restore S4586 } private Task ScheduleHalfOpenTask(ResilienceContext context) { - _executor.ScheduleTask(() => _onHalfOpen!(new OnCircuitHalfOpenedArguments(context)).AsTask(), context, out var task); - return task; + return _executor.ScheduleTask(() => _onHalfOpen!(new OnCircuitHalfOpenedArguments(context)).AsTask()); } } diff --git a/src/Polly.Core/CircuitBreaker/Controller/ScheduledTaskExecutor.cs b/src/Polly.Core/CircuitBreaker/Controller/ScheduledTaskExecutor.cs index a379949f38c..f23e450e7cd 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/ScheduledTaskExecutor.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/ScheduledTaskExecutor.cs @@ -15,7 +15,7 @@ internal sealed class ScheduledTaskExecutor : IDisposable public Task ProcessingTask { get; } - public void ScheduleTask(Func taskFactory, ResilienceContext context, out Task task) + public Task ScheduleTask(Func taskFactory) { #if NET8_0_OR_GREATER ObjectDisposedException.ThrowIf(_disposed, this); @@ -27,10 +27,10 @@ public void ScheduleTask(Func taskFactory, ResilienceContext context, out #endif var source = new TaskCompletionSource(); - task = source.Task; - _tasks.Enqueue(new Entry(taskFactory, context.ContinueOnCapturedContext, source)); + _tasks.Enqueue(new Entry(taskFactory, source)); _semaphore.Release(); + return source.Task; } public void Dispose() @@ -53,36 +53,29 @@ public void Dispose() private async Task StartProcessingAsync() { - while (true) + while (!_disposed) { await _semaphore.WaitAsync().ConfigureAwait(false); - if (_disposed) + if (_disposed || !_tasks.TryDequeue(out var entry)) { return; } - _ = _tasks.TryDequeue(out var entry); - try { - await entry!.TaskFactory().ConfigureAwait(entry.ContinueOnCapturedContext); - entry.TaskCompletion.SetResult(null!); + await entry.TaskFactory().ConfigureAwait(false); + entry.TaskCompletion.TrySetResult(null!); } catch (OperationCanceledException) { - entry!.TaskCompletion.SetCanceled(); + entry.TaskCompletion.TrySetCanceled(); } catch (Exception e) { - entry!.TaskCompletion.SetException(e); - } - - if (_disposed) - { - return; + entry.TaskCompletion.TrySetException(e); } } } - private sealed record Entry(Func TaskFactory, bool ContinueOnCapturedContext, TaskCompletionSource TaskCompletion); + private sealed record Entry(Func TaskFactory, TaskCompletionSource TaskCompletion); } diff --git a/src/Polly.Core/Fallback/FallbackHandler.cs b/src/Polly.Core/Fallback/FallbackHandler.cs index 9d0cfde7ad3..52815e894d9 100644 --- a/src/Polly.Core/Fallback/FallbackHandler.cs +++ b/src/Polly.Core/Fallback/FallbackHandler.cs @@ -2,8 +2,4 @@ namespace Polly.Fallback; internal sealed record class FallbackHandler( Func, ValueTask> ShouldHandle, - Func, ValueTask>> ActionGenerator) -{ - public ValueTask> GetFallbackOutcomeAsync(FallbackActionArguments args) => ActionGenerator(args); -} - + Func, ValueTask>> ActionGenerator); diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs index 5fb5d4ba019..4fed52e0452 100644 --- a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs +++ b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs @@ -19,7 +19,16 @@ public FallbackResilienceStrategy(FallbackHandler handler, Func> ExecuteCore(Func>> callback, ResilienceContext context, TState state) { - var outcome = await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + Outcome outcome; + try + { + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } + catch (Exception ex) + { + outcome = new(ex); + } + var handleFallbackArgs = new FallbackPredicateArguments(context, outcome); if (!await _handler.ShouldHandle(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext)) { @@ -37,7 +46,7 @@ protected internal override async ValueTask> ExecuteCore(Func try { - return await _handler.GetFallbackOutcomeAsync(new FallbackActionArguments(context, outcome)).ConfigureAwait(context.ContinueOnCapturedContext); + return await _handler.ActionGenerator(new FallbackActionArguments(context, outcome)).ConfigureAwait(context.ContinueOnCapturedContext); } catch (Exception e) { diff --git a/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs b/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs index 42945860cfb..10a677b9658 100644 --- a/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs +++ b/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs @@ -120,14 +120,19 @@ public async ValueTask DisposeAsync() return TryRemoveExecutedTask(); } - using var delayTaskCancellation = CancellationTokenSource.CreateLinkedTokenSource(PrimaryContext!.CancellationToken); - #if NET8_0_OR_GREATER - var delayTask = Task.Delay(hedgingDelay, _timeProvider, delayTaskCancellation.Token); + var whenAnyHedgedTask = WaitForTaskCompetitionAsync(); + await whenAnyHedgedTask.WaitAsync(hedgingDelay, _timeProvider, PrimaryContext!.CancellationToken) + .ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing | (ContinueOnCapturedContext ? ConfigureAwaitOptions.ContinueOnCapturedContext : 0)); + + if (!whenAnyHedgedTask.IsCompleted) + { + return null; + } #else + using var delayTaskCancellation = CancellationTokenSource.CreateLinkedTokenSource(PrimaryContext!.CancellationToken); var delayTask = _timeProvider.Delay(hedgingDelay, delayTaskCancellation.Token); -#endif - Task whenAnyHedgedTask = WaitForTaskCompetitionAsync(); + var whenAnyHedgedTask = WaitForTaskCompetitionAsync(); var completedTask = await Task.WhenAny(whenAnyHedgedTask, delayTask).ConfigureAwait(ContinueOnCapturedContext); if (completedTask == delayTask) @@ -135,16 +140,9 @@ public async ValueTask DisposeAsync() return null; } - // cancel the ongoing delay task - // Stryker disable once boolean : no means to test this -#if NET8_0_OR_GREATER - await delayTaskCancellation.CancelAsync().ConfigureAwait(ContinueOnCapturedContext); -#else - delayTaskCancellation.Cancel(throwOnFirstException: false); + delayTaskCancellation.Cancel(); #endif - await whenAnyHedgedTask.ConfigureAwait(ContinueOnCapturedContext); - return TryRemoveExecutedTask(); } @@ -162,30 +160,25 @@ private ExecutionInfo CreateExecutionInfoWhenNoExecution() return new ExecutionInfo(null, false, null); } - private Task WaitForTaskCompetitionAsync() + private Task WaitForTaskCompetitionAsync() { #pragma warning disable S109 // Magic numbers should not be used return _executingTasks.Count switch { - 1 => AwaitTask(_executingTasks[0], ContinueOnCapturedContext), + 1 => _executingTasks[0].ExecutionTaskSafe!, 2 => Task.WhenAny(_executingTasks[0].ExecutionTaskSafe!, _executingTasks[1].ExecutionTaskSafe!), _ => Task.WhenAny(_executingTasks.Select(v => v.ExecutionTaskSafe!)) }; #pragma warning restore S109 // Magic numbers should not be used - - static async Task AwaitTask(TaskExecution task, bool continueOnCapturedContext) - { - // ExecutionTask never fails - await task.ExecutionTaskSafe!.ConfigureAwait(continueOnCapturedContext); - return Task.FromResult(task); - } } private TaskExecution? TryRemoveExecutedTask() { - if (_executingTasks.Find(static v => v.ExecutionTaskSafe!.IsCompleted) is TaskExecution execution) + var i = _executingTasks.FindIndex(static v => v.ExecutionTaskSafe!.IsCompleted); + if (i != -1) { - _executingTasks.Remove(execution); + var execution = _executingTasks[i]; + _executingTasks.RemoveAt(i); return execution; } @@ -194,12 +187,7 @@ static async Task AwaitTask(TaskExecution task, bool continueOnCaptured private void UpdateOriginalContext() { - if (LoadedTasks == 0) - { - return; - } - - if (Tasks.FirstOrDefault(static t => t.IsAccepted) is TaskExecution acceptedExecution) + if (_tasks.Find(static t => t.IsAccepted) is TaskExecution acceptedExecution) { PrimaryContext!.Properties.AddOrReplaceProperties(acceptedExecution.Context.Properties); } diff --git a/src/Polly.Core/Hedging/Controller/HedgingHandler.cs b/src/Polly.Core/Hedging/Controller/HedgingHandler.cs index e5839ae6d95..c5d1ca3a551 100644 --- a/src/Polly.Core/Hedging/Controller/HedgingHandler.cs +++ b/src/Polly.Core/Hedging/Controller/HedgingHandler.cs @@ -5,15 +5,5 @@ internal sealed record class HedgingHandler( Func, Func>>?> ActionGenerator, Func, ValueTask>? OnHedging) { - public Func>>? GenerateAction(HedgingActionGeneratorArguments args) - { - var copiedArgs = new HedgingActionGeneratorArguments( - args.PrimaryContext, - args.ActionContext, - args.AttemptNumber, - args.Callback); - - return ActionGenerator(copiedArgs); - } + public readonly bool IsDefaultActionGenerator = ActionGenerator == HedgingStrategyOptions.DefaultActionGenerator; } - diff --git a/src/Polly.Core/Hedging/Controller/TaskExecution.cs b/src/Polly.Core/Hedging/Controller/TaskExecution.cs index 354b7db27bd..d7e7ee6e0cb 100644 --- a/src/Polly.Core/Hedging/Controller/TaskExecution.cs +++ b/src/Polly.Core/Hedging/Controller/TaskExecution.cs @@ -107,26 +107,34 @@ public async ValueTask InitializeAsync( if (type == HedgedTaskType.Secondary) { Func>>? action = null; - - try + if (!_handler.IsDefaultActionGenerator) { - action = _handler.GenerateAction(CreateArguments(primaryCallback, primaryContext, state, attemptNumber)); - if (action == null) + try + { + action = _handler.ActionGenerator(CreateArguments(primaryCallback, primaryContext, state, attemptNumber)); + if (action == null) + { + await ResetAsync().ConfigureAwait(false); + return false; + } + } + catch (Exception e) { - await ResetAsync().ConfigureAwait(false); - return false; + _stopExecutionTimestamp = _timeProvider.GetTimestamp(); + ExecutionTaskSafe = UpdateOutcomeAsync(new(e)); + return true; } } - catch (Exception e) + + var args = new OnHedgingArguments(primaryContext, Context, attemptNumber - 1); + _telemetry.Report(new(ResilienceEventSeverity.Warning, HedgingConstants.OnHedgingEventName), Context, args); + + if (_handler.OnHedging is { } onHedging) { - _stopExecutionTimestamp = _timeProvider.GetTimestamp(); - ExecutionTaskSafe = ExecuteCreateActionException(e); - return true; + await onHedging(args).ConfigureAwait(Context.ContinueOnCapturedContext); } - await HandleOnHedgingAsync(primaryContext, attemptNumber - 1).ConfigureAwait(Context.ContinueOnCapturedContext); - - ExecutionTaskSafe = ExecuteSecondaryActionAsync(action); + ExecutionTaskSafe = ExecuteSecondaryActionAsync(action, primaryCallback, state, primaryContext.IsSynchronous); } else { @@ -136,23 +144,8 @@ public async ValueTask InitializeAsync( return true; } - private async Task HandleOnHedgingAsync(ResilienceContext primaryContext, int attemptNumber) - { - var args = new OnHedgingArguments( - primaryContext, - Context, - attemptNumber); - - _telemetry.Report(new(ResilienceEventSeverity.Warning, HedgingConstants.OnHedgingEventName), Context, args); - - if (_handler.OnHedging is { } onHedging) - { - await onHedging(args).ConfigureAwait(Context.ContinueOnCapturedContext); - } - } - - private HedgingActionGeneratorArguments CreateArguments( - Func>> primaryCallback, + private HedgingActionGeneratorArguments CreateArguments( + Func>> primaryCallback, ResilienceContext primaryContext, TState state, int attempt) => new(primaryContext, Context, attempt, (context) => primaryCallback(context, state)); @@ -200,24 +193,30 @@ public async ValueTask ResetAsync() } [DebuggerDisableUserUnhandledExceptions] - private async Task ExecuteSecondaryActionAsync(Func>> action) + private async Task ExecuteSecondaryActionAsync( + Func>>? action, + Func>> primaryCallback, + TState state, + bool isSynchronous) { Outcome outcome; try { - outcome = await action().ConfigureAwait(Context.ContinueOnCapturedContext); + var task = action?.Invoke() ?? (isSynchronous ? ExecuteSecondaryActionSync(primaryCallback, state) : primaryCallback(Context, state)); + outcome = await task.ConfigureAwait(Context.ContinueOnCapturedContext); } catch (Exception e) { - outcome = Polly.Outcome.FromException(e); + outcome = new(e); } _stopExecutionTimestamp = _timeProvider.GetTimestamp(); await UpdateOutcomeAsync(outcome).ConfigureAwait(Context.ContinueOnCapturedContext); } - private async Task ExecuteCreateActionException(Exception e) => await UpdateOutcomeAsync(Polly.Outcome.FromException(e)).ConfigureAwait(Context.ContinueOnCapturedContext); + private ValueTask> ExecuteSecondaryActionSync(Func>> primaryCallback, TState state) + => new(Task.Run(() => primaryCallback(Context, state).AsTask())); [DebuggerDisableUserUnhandledExceptions] private async Task ExecutePrimaryActionAsync(Func>> primaryCallback, TState state) @@ -230,7 +229,7 @@ private async Task ExecutePrimaryActionAsync(Func(e); + outcome = new(e); } _stopExecutionTimestamp = _timeProvider.GetTimestamp(); diff --git a/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs b/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs index d020a4d0777..962b7d62fa2 100644 --- a/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs +++ b/src/Polly.Core/Hedging/HedgingResilienceStrategy.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using Polly.Hedging.Utils; using Polly.Telemetry; @@ -31,7 +30,6 @@ public HedgingResilienceStrategy( public HedgingHandler HedgingHandler { get; } - [ExcludeFromCodeCoverage] // coverlet issue protected internal override async ValueTask> ExecuteCore( Func>> callback, ResilienceContext context, @@ -42,7 +40,41 @@ protected internal override async ValueTask> ExecuteCore( try { - return await ExecuteCoreAsync(hedgingContext, callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + // Capture the original cancellation token so it stays the same while hedging is executing. + // If we do not do this the inner strategy can replace the cancellation token and with the concurrent + // nature of hedging this can cause issues. + var cancellationToken = context.CancellationToken; + var continueOnCapturedContext = context.ContinueOnCapturedContext; + + while (true) + { + if (cancellationToken.IsCancellationRequested) + { + return Outcome.FromException(new OperationCanceledException(cancellationToken).TrySetStackTrace()); + } + + var loadedExecution = await hedgingContext.LoadExecutionAsync(callback, state).ConfigureAwait(continueOnCapturedContext); + + if (loadedExecution.Outcome is Outcome outcome) + { + return outcome; + } + + var delay = await GetHedgingDelayAsync(context, hedgingContext.LoadedTasks).ConfigureAwait(continueOnCapturedContext); + var execution = await hedgingContext.TryWaitForCompletedExecutionAsync(delay).ConfigureAwait(continueOnCapturedContext); + if (execution is null) + { + continue; + } + + outcome = execution.Outcome; + + if (!execution.IsHandled) + { + execution.AcceptOutcome(); + return outcome; + } + } } finally { @@ -50,49 +82,6 @@ protected internal override async ValueTask> ExecuteCore( } } - private async ValueTask> ExecuteCoreAsync( - HedgingExecutionContext hedgingContext, - Func>> callback, - ResilienceContext context, - TState state) - { - // Capture the original cancellation token so it stays the same while hedging is executing. - // If we do not do this the inner strategy can replace the cancellation token and with the concurrent - // nature of hedging this can cause issues. - var cancellationToken = context.CancellationToken; - var continueOnCapturedContext = context.ContinueOnCapturedContext; - - while (true) - { - if (cancellationToken.IsCancellationRequested) - { - return Outcome.FromException(new OperationCanceledException(cancellationToken).TrySetStackTrace()); - } - - var loadedExecution = await hedgingContext.LoadExecutionAsync(callback, state).ConfigureAwait(context.ContinueOnCapturedContext); - - if (loadedExecution.Outcome is Outcome outcome) - { - return outcome; - } - - var delay = await GetHedgingDelayAsync(context, hedgingContext.LoadedTasks).ConfigureAwait(continueOnCapturedContext); - var execution = await hedgingContext.TryWaitForCompletedExecutionAsync(delay).ConfigureAwait(continueOnCapturedContext); - if (execution is null) - { - continue; - } - - outcome = execution.Outcome; - - if (!execution.IsHandled) - { - execution.AcceptOutcome(); - return outcome; - } - } - } - internal ValueTask GetHedgingDelayAsync(ResilienceContext context, int attempt) { if (DelayGenerator == null) diff --git a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs index 49aea2ccba8..2874d85d82e 100644 --- a/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs +++ b/src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs @@ -52,16 +52,18 @@ public class HedgingStrategyOptions : ResilienceStrategyOptions /// The default generator executes the original callback that was passed to the hedging resilience strategy. This property is required. /// [Required] - public Func, Func>>?> ActionGenerator { get; set; } = args => + public Func, Func>>?> ActionGenerator { get; set; } = DefaultActionGenerator; + + internal static readonly Func, Func>>?> DefaultActionGenerator = args => { - return async () => + return () => { if (args.PrimaryContext.IsSynchronous) { - return await Task.Run(() => args.Callback(args.ActionContext).AsTask()).ConfigureAwait(args.ActionContext.ContinueOnCapturedContext); + return new(Task.Run(() => args.Callback(args.ActionContext).AsTask())); } - return await args.Callback(args.ActionContext).ConfigureAwait(args.ActionContext.ContinueOnCapturedContext); + return args.Callback(args.ActionContext); }; }; diff --git a/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs b/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs index 848cd6eaeeb..55d9323aae7 100644 --- a/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs +++ b/src/Polly.Core/Registry/RegistryPipelineComponentBuilder.cs @@ -32,26 +32,25 @@ public RegistryPipelineComponentBuilder( internal (ResilienceContextPool? ContextPool, PipelineComponent Component) CreateComponent() { - var builder = CreateBuilder(); - var component = builder.ComponentFactory(); + var (component, reloadTokens, telemetry, instance) = CreateBuilder(); - if (builder.ReloadTokens.Count == 0) + if (reloadTokens.Count == 0) { - return (builder.Instance.ContextPool, component); + return (instance.ContextPool, component); } component = PipelineComponentFactory.CreateReloadable( - new ReloadableComponent.Entry(component, builder.ReloadTokens, builder.Telemetry), + new ReloadableComponent.Entry(component, reloadTokens, telemetry), () => { - var builder = CreateBuilder(); - return new ReloadableComponent.Entry(builder.ComponentFactory(), builder.ReloadTokens, builder.Telemetry); + var (component, reloadTokens, telemetry, _) = CreateBuilder(); + return new ReloadableComponent.Entry(component, reloadTokens, telemetry); }); - return (builder.Instance.ContextPool, component); + return (instance.ContextPool, component); } - private Builder CreateBuilder() + private (PipelineComponent Component, List ReloadTokens, ResilienceStrategyTelemetry Telemetry, TBuilder Instance) CreateBuilder() { var context = new ConfigureBuilderContext(_key, _builderName, _instanceName); var builder = _activator(); @@ -64,20 +63,9 @@ private Builder CreateBuilder() new ResilienceTelemetrySource(builder.Name, builder.InstanceName, null), builder.TelemetryListener); - return new( - () => - { - var innerComponent = PipelineComponentFactory.WithDisposableCallbacks(builder.BuildPipelineComponent(), context.DisposeCallbacks); - return PipelineComponentFactory.WithExecutionTracking(innerComponent, timeProvider); - }, - context.ReloadTokens, - telemetry, - builder); - } + var innerComponent = PipelineComponentFactory.WithDisposableCallbacks(builder.BuildPipelineComponent(), context.DisposeCallbacks); + var component = PipelineComponentFactory.WithExecutionTracking(innerComponent, timeProvider); - private sealed record Builder( - Func ComponentFactory, - List ReloadTokens, - ResilienceStrategyTelemetry Telemetry, - TBuilder Instance); + return new(component, context.ReloadTokens, telemetry, builder); + } } diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs index 847ae57e182..0d09188dd79 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.TResult.cs @@ -47,33 +47,31 @@ public bool TryGet(TKey key, [NotNullWhen(true)] out ResiliencePipeline public ResiliencePipeline GetOrAdd(TKey key, Action, ConfigureBuilderContext> configure) { - var context = new ConfigureBuilderContext(key, _builderNameFormatter(key), _instanceNameFormatter?.Invoke(key)); - - return _pipelines.GetOrAdd(key, k => + if (_pipelines.TryGetValue(key, out var pipeline)) { - var componentBuilder = new RegistryPipelineComponentBuilder, TKey>( - _activator, - k, - _builderNameFormatter(k), - _instanceNameFormatter?.Invoke(k), - configure); + return pipeline; + } - (var contextPool, var component) = componentBuilder.CreateComponent(); + var componentBuilder = new RegistryPipelineComponentBuilder, TKey>( + _activator, + key, + _builderNameFormatter(key), + _instanceNameFormatter?.Invoke(key), + configure); - return new ResiliencePipeline(component, DisposeBehavior.Reject, contextPool); - }); + (var contextPool, var component) = componentBuilder.CreateComponent(); + + return _pipelines.GetOrAdd(key, new ResiliencePipeline(component, DisposeBehavior.Reject, contextPool)); } public bool TryAddBuilder(TKey key, Action, ConfigureBuilderContext> configure) => _builders.TryAdd(key, configure); public async ValueTask DisposeAsync() { - foreach (var strategy in _pipelines.Values) + foreach (var kv in _pipelines) { - await strategy.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); + await kv.Value.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); } - - _pipelines.Clear(); } } } diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs index 4b35fc69297..db55ee40ff8 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs @@ -122,20 +122,16 @@ public ResiliencePipeline GetOrAddPipeline(TKey key, Action - { - var componentBuilder = new RegistryPipelineComponentBuilder( - _activator, - k, - _builderNameFormatter(k), - _instanceNameFormatter?.Invoke(k), - configure) - ; - - (var contextPool, var component) = componentBuilder.CreateComponent(); - - return new ResiliencePipeline(component, DisposeBehavior.Reject, contextPool); - }); + var componentBuilder = new RegistryPipelineComponentBuilder( + _activator, + key, + _builderNameFormatter(key), + _instanceNameFormatter?.Invoke(key), + configure); + + (var contextPool, var component) = componentBuilder.CreateComponent(); + + return _pipelines.GetOrAdd(key, new ResiliencePipeline(component, DisposeBehavior.Reject, contextPool)); } /// @@ -234,20 +230,14 @@ 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) + foreach (var kv in _pipelines) { - await pipeline.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); + await kv.Value.DisposeHelper.ForceDisposeAsync().ConfigureAwait(false); } - foreach (var disposable in registries) + foreach (var kv in _genericRegistry) { - await disposable.DisposeAsync().ConfigureAwait(false); + await ((IAsyncDisposable)kv.Value).DisposeAsync().ConfigureAwait(false); } } @@ -258,15 +248,12 @@ private GenericRegistry GetGenericRegistry() return (GenericRegistry)genericRegistry; } - return (GenericRegistry)_genericRegistry.GetOrAdd(typeof(TResult), _ => - { - return new GenericRegistry( - () => new ResiliencePipelineBuilder(_activator()), - _builderComparer, - _pipelineComparer, - _builderNameFormatter, - _instanceNameFormatter); - }); + return (GenericRegistry)_genericRegistry.GetOrAdd(typeof(TResult), new GenericRegistry( + () => new ResiliencePipelineBuilder(_activator()), + _builderComparer, + _pipelineComparer, + _builderNameFormatter, + _instanceNameFormatter)); } private void EnsureNotDisposed() diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs index 63221f2aaa1..c4b78fcfb49 100644 --- a/src/Polly.Core/Retry/RetryResilienceStrategy.cs +++ b/src/Polly.Core/Retry/RetryResilienceStrategy.cs @@ -52,7 +52,18 @@ protected internal override async ValueTask> ExecuteCore(Func while (true) { var startTimestamp = _timeProvider.GetTimestamp(); - var outcome = await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + Outcome outcome; + try + { + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + outcome = new(ex); + } +#pragma warning restore CA1031 + var shouldRetryArgs = new RetryPredicateArguments(context, outcome, attempt); var handle = await ShouldHandle(shouldRetryArgs).ConfigureAwait(context.ContinueOnCapturedContext); var executionTime = _timeProvider.GetElapsedTime(startTimestamp); diff --git a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs index 68c816ba2f3..3997ab0e929 100644 --- a/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs +++ b/src/Polly.Core/Retry/RetryStrategyOptions.TResult.cs @@ -131,5 +131,5 @@ public class RetryStrategyOptions : ResilienceStrategyOptions /// [EditorBrowsable(EditorBrowsableState.Never)] [Required] - public Func Randomizer { get; set; } = RandomUtil.Instance.NextDouble; + public Func Randomizer { get; set; } = RandomUtil.NextDouble; } diff --git a/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs b/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs index 9a22a678820..9c47110f105 100644 --- a/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs +++ b/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs @@ -40,7 +40,17 @@ protected internal override async ValueTask> ExecuteCoreThe instance. /// A boolean value that indicates whether or not the chaos strategy should be injected. /// Use this method before injecting any chaos strategy to evaluate whether a given chaos strategy needs to be injected during the execution. - protected async ValueTask ShouldInjectAsync(ResilienceContext context) => - await ChaosStrategyHelper - .ShouldInjectAsync(context, InjectionRateGenerator, EnabledGenerator, _randomizer) - .ConfigureAwait(false); + protected ValueTask ShouldInjectAsync(ResilienceContext context) => + ChaosStrategyHelper.ShouldInjectAsync(context, InjectionRateGenerator, EnabledGenerator, _randomizer); } diff --git a/src/Polly.Core/Simmy/ChaosStrategyOptions.cs b/src/Polly.Core/Simmy/ChaosStrategyOptions.cs index 79d338111db..fa59460f1f0 100644 --- a/src/Polly.Core/Simmy/ChaosStrategyOptions.cs +++ b/src/Polly.Core/Simmy/ChaosStrategyOptions.cs @@ -47,5 +47,5 @@ public abstract class ChaosStrategyOptions : ResilienceStrategyOptions /// The default randomizer is thread safe and returns values between 0.0 and 1.0. /// [Required] - public Func Randomizer { get; set; } = RandomUtil.Instance.NextDouble; + public Func Randomizer { get; set; } = RandomUtil.NextDouble; } diff --git a/src/Polly.Core/Simmy/Fault/ChaosFaultStrategy.cs b/src/Polly.Core/Simmy/Fault/ChaosFaultStrategy.cs index 6caede5ead6..84140866960 100644 --- a/src/Polly.Core/Simmy/Fault/ChaosFaultStrategy.cs +++ b/src/Polly.Core/Simmy/Fault/ChaosFaultStrategy.cs @@ -43,7 +43,17 @@ protected internal override async ValueTask> ExecuteCore /// Initializes a new instance of the class. /// - public FaultGenerator() - => _helper = new GeneratorHelper(RandomUtil.Instance.Next); + public FaultGenerator() => _helper = new(RandomUtil.Next); /// /// Registers an exception generator delegate. diff --git a/src/Polly.Core/Simmy/Latency/ChaosLatencyStrategy.cs b/src/Polly.Core/Simmy/Latency/ChaosLatencyStrategy.cs index d932b5776b9..553449dfd1b 100644 --- a/src/Polly.Core/Simmy/Latency/ChaosLatencyStrategy.cs +++ b/src/Polly.Core/Simmy/Latency/ChaosLatencyStrategy.cs @@ -37,24 +37,31 @@ protected internal override async ValueTask> ExecuteCore TimeSpan.Zero) { - // do nothing - return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); - } - - var args = new OnLatencyInjectedArguments(context, latency); - _telemetry.Report(new(ResilienceEventSeverity.Information, ChaosLatencyConstants.OnLatencyInjectedEvent), context, args); + var args = new OnLatencyInjectedArguments(context, latency); + _telemetry.Report(new(ResilienceEventSeverity.Information, ChaosLatencyConstants.OnLatencyInjectedEvent), context, args); - await _timeProvider.DelayAsync(latency, context).ConfigureAwait(context.ContinueOnCapturedContext); + await _timeProvider.DelayAsync(latency, context).ConfigureAwait(context.ContinueOnCapturedContext); - if (OnLatencyInjected is not null) - { - await OnLatencyInjected(args).ConfigureAwait(context.ContinueOnCapturedContext); + if (OnLatencyInjected is not null) + { + await OnLatencyInjected(args).ConfigureAwait(context.ContinueOnCapturedContext); + } } } - return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + return await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + return new(ex); + } +#pragma warning restore CA1031 } catch (OperationCanceledException e) { diff --git a/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs b/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs index 137884414ac..5c37b4cd995 100644 --- a/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs +++ b/src/Polly.Core/Simmy/Outcomes/ChaosOutcomeStrategy.cs @@ -34,7 +34,17 @@ await _outcomeGenerator(new(context)).ConfigureAwait(context.ContinueOnCapturedC return outcome; } - return await StrategyHelper.ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext); + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + return await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + return new(ex); + } +#pragma warning restore CA1031 } catch (OperationCanceledException e) { diff --git a/src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs b/src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs index 95d961f3d4c..fc0004d539c 100644 --- a/src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs +++ b/src/Polly.Core/Simmy/Outcomes/OutcomeGenerator.cs @@ -21,13 +21,7 @@ public sealed class OutcomeGenerator /// /// Initializes a new instance of the class. /// - public OutcomeGenerator() - : this(RandomUtil.Instance.Next) - { - } - - internal OutcomeGenerator(Func weightGenerator) - => _helper = new GeneratorHelper(weightGenerator); + public OutcomeGenerator() => _helper = new(RandomUtil.Next); /// /// Registers an exception generator delegate. diff --git a/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs b/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs index 98f570c87fb..53146ab1803 100644 --- a/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs +++ b/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs @@ -45,7 +45,18 @@ protected internal override async ValueTask> ExecuteCore outcome; + try + { + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + outcome = new(ex); + } +#pragma warning restore CA1031 + var isCancellationRequested = cancellationSource.IsCancellationRequested; // execution is finished, clean up diff --git a/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs b/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs index 222e5de7338..2e9ae5336a2 100644 --- a/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs +++ b/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs @@ -18,9 +18,9 @@ internal override ValueTask> ExecuteCore( // Check if we can cast directly, thus saving some cycles and improving the performance if (callback is Func>> casted) { - return ConvertValueTask( - Strategy.ExecuteCore(casted, context, state), - context); +#pragma warning disable CA2012 + return (ValueTask>)(object)Strategy.ExecuteCore(casted, context, state); +#pragma warning restore CA2012 } else { @@ -33,46 +33,18 @@ static async (context, state) => context, (callback, state)); - return ConvertValueTask(valueTask, context); - } - } + if (valueTask.IsCompletedSuccessfully) + { + return new ValueTask>(ConvertOutcome(valueTask.Result)); + } - private static ValueTask> ConvertValueTask(ValueTask> valueTask, ResilienceContext resilienceContext) - { - if (valueTask.IsCompletedSuccessfully) - { - return new ValueTask>(ConvertOutcome(valueTask.Result)); + return ConvertValueTaskAsync(valueTask, context); } - return ConvertValueTaskAsync(valueTask, resilienceContext); - - static async ValueTask> ConvertValueTaskAsync(ValueTask> valueTask, ResilienceContext resilienceContext) + static async ValueTask> ConvertValueTaskAsync(ValueTask> valueTask, ResilienceContext resilienceContext) { var outcome = await valueTask.ConfigureAwait(resilienceContext.ContinueOnCapturedContext); - return ConvertOutcome(outcome); - } - } - - private static Outcome ConvertOutcome(Outcome outcome) - { - if (outcome.ExceptionDispatchInfo is not null) - { - return new Outcome(outcome.ExceptionDispatchInfo); + return ConvertOutcome(outcome); } - - if (outcome.Result is null) - { - return new Outcome(default(TTo)); - } - - if (typeof(TTo) == typeof(TFrom)) - { - var result = outcome.Result; - - // We can use the unsafe cast here because we know for sure these two types are the same - return new Outcome(Unsafe.As(ref result)); - } - - return new Outcome((TTo)(object)outcome.Result); } } diff --git a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs index e5181d53c5f..5b3e1fa09e7 100644 --- a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs +++ b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs @@ -19,4 +19,14 @@ public override ValueTask DisposeAsync() return default; } + + protected static Outcome ConvertOutcome(Outcome outcome) + { + if (outcome.ExceptionDispatchInfo is not null) + { + return new(outcome.ExceptionDispatchInfo); + } + + return outcome.Result is null ? default : new((TTo)(object)outcome.Result); + } } diff --git a/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs b/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs index 093858ae7b2..40ecc6bfda5 100644 --- a/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs +++ b/src/Polly.Core/Utils/Pipeline/PipelineComponentFactory.cs @@ -12,28 +12,8 @@ internal static class PipelineComponentFactory public static PipelineComponent FromStrategy(ResilienceStrategy strategy) => new BridgeComponent(strategy); - public static PipelineComponent WithDisposableCallbacks(PipelineComponent component, IEnumerable callbacks) - { -#pragma warning disable CA1851 // Possible multiple enumerations of 'IEnumerable' collection -#if NET6_0_OR_GREATER - if (callbacks.TryGetNonEnumeratedCount(out var count)) - { - if (count == 0) - { - return component; - } - } - else if (!callbacks.Any()) -#else - if (!callbacks.Any()) -#endif - { - return component; - } - - return new ComponentWithDisposeCallbacks(component, callbacks.ToList()); -#pragma warning restore CA1851 // Possible multiple enumerations of 'IEnumerable' collection - } + public static PipelineComponent WithDisposableCallbacks(PipelineComponent component, List callbacks) + => callbacks.Count == 0 ? component : new ComponentWithDisposeCallbacks(component, callbacks); public static PipelineComponent WithExecutionTracking(PipelineComponent component, TimeProvider timeProvider) => new ExecutionTrackingComponent(component, timeProvider); diff --git a/src/Polly.Core/Utils/RandomUtil.cs b/src/Polly.Core/Utils/RandomUtil.cs index de7bc6ff6ce..4cffd6e3101 100644 --- a/src/Polly.Core/Utils/RandomUtil.cs +++ b/src/Polly.Core/Utils/RandomUtil.cs @@ -1,18 +1,16 @@ namespace Polly.Utils; -#pragma warning disable CA1001 // Types that own disposable fields should be disposable #pragma warning disable CA5394 // Do not use insecure randomness -#pragma warning disable S2931 // Classes with "IDisposable" members should implement "IDisposable" -internal sealed class RandomUtil +internal static class RandomUtil { - private readonly ThreadLocal _random; +#if NET + public static double NextDouble() => Random.Shared.NextDouble(); + public static int Next(int maxValue) => Random.Shared.Next(maxValue); +#else + private static readonly ThreadLocal Instance = new(() => new Random()); - public static readonly RandomUtil Instance = new(null); - - public RandomUtil(int? seed) => _random = new ThreadLocal(() => seed == null ? new Random() : new Random(seed.Value)); - - public double NextDouble() => _random.Value!.NextDouble(); - - public int Next(int maxValue) => _random.Value!.Next(maxValue); + public static double NextDouble() => Instance.Value.NextDouble(); + public static int Next(int maxValue) => Instance.Value.Next(maxValue); +#endif } diff --git a/src/Polly.Core/Utils/StrategyHelper.cs b/src/Polly.Core/Utils/StrategyHelper.cs deleted file mode 100644 index 2b63321b079..00000000000 --- a/src/Polly.Core/Utils/StrategyHelper.cs +++ /dev/null @@ -1,46 +0,0 @@ -namespace Polly.Utils; - -#pragma warning disable CA1031 // Do not catch general exception types - -internal static class StrategyHelper -{ - [DebuggerDisableUserUnhandledExceptions] - public static ValueTask> ExecuteCallbackSafeAsync( - Func>> callback, - ResilienceContext context, - TState state) - { - if (context.CancellationToken.IsCancellationRequested) - { - return new ValueTask>(Outcome.FromException(new OperationCanceledException(context.CancellationToken))); - } - - try - { - var callbackTask = callback(context, state); - if (callbackTask.IsCompleted) - { - return new ValueTask>(callbackTask.GetResult()); - } - - return AwaitTask(callbackTask, context.ContinueOnCapturedContext); - } - catch (Exception e) - { - return new ValueTask>(Outcome.FromException(e)); - } - - [DebuggerDisableUserUnhandledExceptions] - static async ValueTask> AwaitTask(ValueTask> task, bool continueOnCapturedContext) - { - try - { - return await task.ConfigureAwait(continueOnCapturedContext); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - } - } -} diff --git a/src/Polly.RateLimiting/DisposeWrapper.cs b/src/Polly.RateLimiting/DisposeWrapper.cs deleted file mode 100644 index 2a062747eb2..00000000000 --- a/src/Polly.RateLimiting/DisposeWrapper.cs +++ /dev/null @@ -1,14 +0,0 @@ -using System.Threading.RateLimiting; - -namespace Polly.RateLimiting; - -internal sealed class DisposeWrapper : IDisposable, IAsyncDisposable -{ - internal DisposeWrapper(RateLimiter limiter) => Limiter = limiter; - - public RateLimiter Limiter { get; } - - public ValueTask DisposeAsync() => Limiter.DisposeAsync(); - - public void Dispose() => Limiter.Dispose(); -} diff --git a/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs b/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs index d81efa31f94..48aab46f67f 100644 --- a/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs +++ b/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs @@ -107,12 +107,12 @@ public static TBuilder AddRateLimiter( return builder.AddStrategy( context => { - DisposeWrapper? wrapper = default; + RateLimiter? wrapper = default; var limiter = options.RateLimiter; if (limiter is null) { var defaultLimiter = new ConcurrencyLimiter(options.DefaultRateLimiterOptions); - wrapper = new DisposeWrapper(defaultLimiter); + wrapper = defaultLimiter; limiter = args => defaultLimiter.AcquireAsync(cancellationToken: args.Context.CancellationToken); } diff --git a/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs b/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs index f7431e571f6..20c819bf24e 100644 --- a/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs +++ b/src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs @@ -11,7 +11,7 @@ public RateLimiterResilienceStrategy( Func> limiter, Func? onRejected, ResilienceStrategyTelemetry telemetry, - DisposeWrapper? wrapper) + RateLimiter? wrapper) { Limiter = limiter; OnLeaseRejected = onRejected; @@ -24,7 +24,7 @@ public RateLimiterResilienceStrategy( public Func? OnLeaseRejected { get; } - public DisposeWrapper? Wrapper { get; } + public RateLimiter? Wrapper { get; } public void Dispose() => Wrapper?.Dispose(); diff --git a/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs b/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs index 569f5f8a4d1..eb1185196af 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/Controller/CircuitStateControllerTests.cs @@ -554,7 +554,7 @@ public async Task ExecuteScheduledTask_Async_Ok() var context = ResilienceContextPool.Shared.Get(cancellationToken); var source = new TaskCompletionSource(); - var task = CircuitStateController.ExecuteScheduledTaskAsync(source.Task, context.Initialize(isSynchronous: false)).AsTask(); + var task = CircuitStateController.ExecuteScheduledTaskAsync(source.Task, context.Initialize(isSynchronous: false)); #pragma warning disable xUnit1031 // Do not use blocking task operations in test method task.Wait(3, cancellationToken).ShouldBeFalse(); diff --git a/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs b/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs index d2e4ab2fcaf..87a18df798a 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs @@ -4,21 +4,17 @@ namespace Polly.Core.Tests.CircuitBreaker.Controller; public class ScheduledTaskExecutorTests { - private static CancellationToken CancellationToken => CancellationToken.None; - [Fact] public async Task ScheduleTask_Success_EnsureExecuted() { using var scheduler = new ScheduledTaskExecutor(); var executed = false; - scheduler.ScheduleTask( + var task = scheduler.ScheduleTask( () => { executed = true; return Task.CompletedTask; - }, - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + }); await task; @@ -29,10 +25,8 @@ public async Task ScheduleTask_Success_EnsureExecuted() public async Task ScheduleTask_OperationCanceledException_EnsureExecuted() { using var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask( - () => throw new OperationCanceledException(), - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + var task = scheduler.ScheduleTask( + () => throw new OperationCanceledException()); await Should.ThrowAsync(() => task); } @@ -41,10 +35,8 @@ public async Task ScheduleTask_OperationCanceledException_EnsureExecuted() public async Task ScheduleTask_Exception_EnsureExecuted() { using var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask( - () => throw new InvalidOperationException(), - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + var task = scheduler.ScheduleTask( + () => throw new InvalidOperationException()); await Should.ThrowAsync(() => task); } @@ -56,22 +48,20 @@ public async Task ScheduleTask_Multiple_EnsureExecutionSerialized() using var verified = new ManualResetEvent(false); using var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask( + var task = scheduler.ScheduleTask( () => { executing.Set(); verified.WaitOne(); return Task.CompletedTask; - }, - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + }); executing.WaitOne(); - scheduler.ScheduleTask(() => Task.CompletedTask, ResilienceContextPool.Shared.Get(CancellationToken), out var otherTask); + var otherTask = scheduler.ScheduleTask(() => Task.CompletedTask); #pragma warning disable xUnit1031 // Do not use blocking task operations in test method - otherTask.Wait(50, CancellationToken).ShouldBeFalse(); + otherTask.Wait(50).ShouldBeFalse(); #pragma warning restore xUnit1031 // Do not use blocking task operations in test method verified.Set(); @@ -87,18 +77,16 @@ public async Task Dispose_ScheduledTaskCancelled() using var verified = new ManualResetEvent(false); var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask( + var task = scheduler.ScheduleTask( () => { executing.Set(); verified.WaitOne(); return Task.CompletedTask; - }, - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + }); executing.WaitOne(); - scheduler.ScheduleTask(() => Task.CompletedTask, ResilienceContextPool.Shared.Get(CancellationToken), out var otherTask); + var otherTask = scheduler.ScheduleTask(() => Task.CompletedTask); scheduler.Dispose(); verified.Set(); await task; @@ -106,7 +94,7 @@ public async Task Dispose_ScheduledTaskCancelled() await Should.ThrowAsync(() => otherTask); Should.Throw( - () => scheduler.ScheduleTask(() => Task.CompletedTask, ResilienceContextPool.Shared.Get(CancellationToken), out _)); + () => scheduler.ScheduleTask(() => Task.CompletedTask)); } [Fact] @@ -118,26 +106,20 @@ public void Dispose_WhenScheduledTaskExecuting() using var ready = new ManualResetEvent(false); var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask( + var task = scheduler.ScheduleTask( () => { ready.Set(); disposed.WaitOne(); return Task.CompletedTask; - }, - ResilienceContextPool.Shared.Get(CancellationToken), - out var task); + }); ready.WaitOne(timeout).ShouldBeTrue(); scheduler.Dispose(); disposed.Set(); #pragma warning disable xUnit1031 -#if NET - scheduler.ProcessingTask.Wait(timeout, CancellationToken).ShouldBeTrue(); -#else scheduler.ProcessingTask.Wait(timeout).ShouldBeTrue(); -#endif #pragma warning restore xUnit1031 } @@ -145,7 +127,7 @@ public void Dispose_WhenScheduledTaskExecuting() public async Task Dispose_EnsureNoBackgroundProcessing() { var scheduler = new ScheduledTaskExecutor(); - scheduler.ScheduleTask(() => Task.CompletedTask, ResilienceContextPool.Shared.Get(CancellationToken), out var otherTask); + var otherTask = scheduler.ScheduleTask(() => Task.CompletedTask); await otherTask; scheduler.Dispose(); #pragma warning disable S3966 // Objects should not be disposed more than once diff --git a/test/Polly.Core.Tests/Fallback/FallbackHandlerTests.cs b/test/Polly.Core.Tests/Fallback/FallbackHandlerTests.cs index fa6330939d6..f722ee5c7c8 100644 --- a/test/Polly.Core.Tests/Fallback/FallbackHandlerTests.cs +++ b/test/Polly.Core.Tests/Fallback/FallbackHandlerTests.cs @@ -8,7 +8,7 @@ public async Task GenerateAction_Generic_Ok() { var handler = FallbackHelper.CreateHandler(_ => true, () => Outcome.FromResult("secondary")); var context = ResilienceContextPool.Shared.Get(); - var outcome = await handler.GetFallbackOutcomeAsync(new FallbackActionArguments(context, Outcome.FromResult("primary")))!; + var outcome = await handler.ActionGenerator(new FallbackActionArguments(context, Outcome.FromResult("primary")))!; outcome.Result.ShouldBe("secondary"); } diff --git a/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs b/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs deleted file mode 100644 index a1866a854e9..00000000000 --- a/test/Polly.Core.Tests/Hedging/HedgingHandlerTests.cs +++ /dev/null @@ -1,33 +0,0 @@ -using Polly.Hedging; -using Polly.Hedging.Utils; - -namespace Polly.Core.Tests.Hedging; - -public static class HedgingHandlerTests -{ - [Fact] - public static async Task GenerateAction_Generic_Ok() - { - // Arrange - var context = ResilienceContextPool.Shared.Get(); - - var handler = new HedgingHandler( - args => PredicateResult.True(), - args => () => Outcome.FromResultAsValueTask("ok"), - args => default); - - handler.OnHedging.ShouldNotBeNull(); - - var action = handler.GenerateAction(new HedgingActionGeneratorArguments( - context, - context, - 0, - _ => Outcome.FromResultAsValueTask("primary")))!; - - // Act - var res = await action(); - - // Assert - res.Result.ShouldBe("ok"); - } -} diff --git a/test/Polly.Core.Tests/Retry/RetryHelperTests.cs b/test/Polly.Core.Tests/Retry/RetryHelperTests.cs index fd0e7358284..8923e8e5576 100644 --- a/test/Polly.Core.Tests/Retry/RetryHelperTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryHelperTests.cs @@ -6,7 +6,7 @@ namespace Polly.Core.Tests.Retry; public class RetryHelperTests { - private Func _randomizer = new RandomUtil(0).NextDouble; + private Func _randomizer = new Random(0).NextDouble; public static TheoryData Attempts() #pragma warning disable IDE0028 @@ -204,7 +204,7 @@ public void GetRetryDelay_Exponential_Is_Positive_When_No_Maximum_Delay(int atte var baseDelay = TimeSpan.FromSeconds(2); TimeSpan? maxDelay = null; - var random = new RandomUtil(0).NextDouble; + var random = new Random(0).NextDouble; double state = 0; var first = RetryHelper.GetRetryDelay(type, jitter, attempt, baseDelay, maxDelay, ref state, random); @@ -224,7 +224,7 @@ public void GetRetryDelay_Exponential_Does_Not_Exceed_MaxDelay(int attempt) var baseDelay = TimeSpan.FromSeconds(2); var maxDelay = TimeSpan.FromSeconds(30); - var random = new RandomUtil(0).NextDouble; + var random = new Random(0).NextDouble; double state = 0; var first = RetryHelper.GetRetryDelay(type, jitter, attempt, baseDelay, maxDelay, ref state, random); @@ -254,8 +254,8 @@ public void ExponentialWithJitter_Ok(int count) public void ExponentialWithJitter_EnsureRandomness() { var delay = TimeSpan.FromSeconds(7.8); - var delays1 = GetExponentialWithJitterBackoff(false, delay, 100, RandomUtil.Instance.NextDouble); - var delays2 = GetExponentialWithJitterBackoff(false, delay, 100, RandomUtil.Instance.NextDouble); + var delays1 = GetExponentialWithJitterBackoff(false, delay, 100, RandomUtil.NextDouble); + var delays2 = GetExponentialWithJitterBackoff(false, delay, 100, RandomUtil.NextDouble); delays1.SequenceEqual(delays2).ShouldBeFalse(); delays1.ShouldAllBe(delay => delay > TimeSpan.Zero); @@ -268,7 +268,7 @@ private static IReadOnlyList GetExponentialWithJitterBackoff(bool cont return Backoff.DecorrelatedJitterBackoffV2(baseDelay, retryCount, 0, false).Take(retryCount).ToArray(); } - var random = randomizer ?? new RandomUtil(0).NextDouble; + var random = randomizer ?? new Random(0).NextDouble; double state = 0; var result = new List(); diff --git a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs index 3fd071fd781..9e849da60ff 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs @@ -29,7 +29,7 @@ public class PipelineComponentFactoryTests public void WithDisposableCallbacks_NoCallbacks_ReturnsOriginalComponent(IEnumerable callbacks) { var component = Substitute.For(); - var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks.ToList()); result.ShouldBeSameAs(component); } @@ -41,7 +41,7 @@ public void PipelineComponentFactory_Should_Return_WrapperComponent_With_Callbac { var component = Substitute.For(); - var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks.ToList()); result.ShouldBeOfType(); } diff --git a/test/Polly.Core.Tests/Utils/RandomUtilTests.cs b/test/Polly.Core.Tests/Utils/RandomUtilTests.cs index 89c2f6f128f..6d8824ec8cb 100644 --- a/test/Polly.Core.Tests/Utils/RandomUtilTests.cs +++ b/test/Polly.Core.Tests/Utils/RandomUtilTests.cs @@ -4,18 +4,9 @@ namespace Polly.Core.Tests.Utils; public class RandomUtilTests { - [InlineData(null)] - [InlineData(0)] - [InlineData(1)] - [Theory] - public void Ctor_Ok(int? seed) - { - var util = new RandomUtil(seed); - - Should.NotThrow(util.NextDouble); - } + [Fact] + public void NextDouble_Ok() => Should.NotThrow(RandomUtil.NextDouble); [Fact] - public void Instance_Ok() => - RandomUtil.Instance.ShouldNotBeNull(); + public void Next_Ok() => Should.NotThrow(() => RandomUtil.Next(42)); } diff --git a/test/Polly.Core.Tests/Utils/StrategyHelperTests.cs b/test/Polly.Core.Tests/Utils/StrategyHelperTests.cs deleted file mode 100644 index 92189c317b2..00000000000 --- a/test/Polly.Core.Tests/Utils/StrategyHelperTests.cs +++ /dev/null @@ -1,65 +0,0 @@ -using Polly.Utils; - -namespace Polly.Core.Tests.Utils; - -public static class StrategyHelperTests -{ - [Fact] - public static async Task ExecuteCallbackSafeAsync_Cancelled_EnsureOperationCanceledException() - { - using var token = new CancellationTokenSource(); - token.Cancel(); - - var outcome = await StrategyHelper.ExecuteCallbackSafeAsync( - (_, _) => throw new InvalidOperationException(), - ResilienceContextPool.Shared.Get(token.Token), - "dummy"); - - outcome.Exception.ShouldBeOfType(); - } - - [Theory] - [InlineData(true)] - [InlineData(false)] - public static async Task ExecuteCallbackSafeAsync_CallbackThrows_EnsureExceptionWrapped(bool isAsync) => - await TestUtilities.AssertWithTimeoutAsync(async () => - { - var outcome = await StrategyHelper.ExecuteCallbackSafeAsync( - async (_, _) => - { - if (isAsync) - { - await Task.Delay(15); - } - - throw new InvalidOperationException(); - }, - ResilienceContextPool.Shared.Get(), - "dummy"); - - outcome.Exception.ShouldBeOfType(); - }); - - [Theory] - [InlineData(true)] - [InlineData(false)] - public static async Task ExecuteCallbackSafeAsync_AsyncCallback_CompletedOk(bool isAsync) => - await TestUtilities.AssertWithTimeoutAsync(async () => - { - var outcomeTask = StrategyHelper.ExecuteCallbackSafeAsync( - async (_, _) => - { - if (isAsync) - { - await Task.Delay(15); - } - - return Outcome.FromResult("success"); - }, - ResilienceContextPool.Shared.Get(), - "dummy"); - - outcomeTask.IsCompleted.ShouldBe(!isAsync); - (await outcomeTask).Result.ShouldBe("success"); - }); -} diff --git a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs index c942a29f114..e17f5705a3b 100644 --- a/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs +++ b/test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensionsTests.cs @@ -15,7 +15,7 @@ public class RateLimiterResiliencePipelineBuilderExtensionsTests builder => { builder.AddConcurrencyLimiter(2, 2); - AssertRateLimiterStrategy(builder, strategy => strategy.Wrapper!.Limiter.ShouldBeOfType()); + AssertRateLimiterStrategy(builder, strategy => strategy.Wrapper.ShouldBeOfType()); }, builder => { @@ -26,7 +26,7 @@ public class RateLimiterResiliencePipelineBuilderExtensionsTests QueueLimit = 2 }); - AssertRateLimiterStrategy(builder, strategy => strategy.Wrapper!.Limiter.ShouldBeOfType()); + AssertRateLimiterStrategy(builder, strategy => strategy.Wrapper.ShouldBeOfType()); }, builder => { diff --git a/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyTests.cs b/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyTests.cs index ae140b5d5e0..50caa7429ff 100644 --- a/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyTests.cs +++ b/test/Polly.RateLimiting.Tests/RateLimiterResilienceStrategyTests.cs @@ -94,8 +94,7 @@ public async Task Execute_LeaseRejected(bool hasEvents, bool hasRetryAfter) public async Task Dispose_DisposableResourcesShouldBeDisposed(bool isAsync) { using var limiter = new ConcurrencyLimiter(new ConcurrencyLimiterOptions { PermitLimit = 1 }); - using var wrapper = new DisposeWrapper(limiter); - var strategy = new RateLimiterResilienceStrategy(null!, null, null!, wrapper); + var strategy = new RateLimiterResilienceStrategy(null!, null, null!, limiter); if (isAsync) { From e14f82bc7da599aa020cdc66f4c325e638d96c1c Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Wed, 9 Jul 2025 03:16:31 +0300 Subject: [PATCH 02/10] Fix --- src/Polly.Core/Registry/ResiliencePipelineRegistry.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs index db55ee40ff8..4c0cd6d3425 100644 --- a/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs +++ b/src/Polly.Core/Registry/ResiliencePipelineRegistry.cs @@ -228,6 +228,11 @@ public bool TryAddBuilder(TKey key, Action public async ValueTask DisposeAsync() { + if (_disposed) + { + return; + } + _disposed = true; foreach (var kv in _pipelines) From 67150d2ebb9869e7568fd1b7272b7379d0fc5da4 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Wed, 13 Aug 2025 23:34:25 +0300 Subject: [PATCH 03/10] Cleanup. Improve test coverage. --- src/Polly.Core/ResiliencePipeline.Sync.cs | 119 +++++------------- src/Polly.Core/ResiliencePipeline.SyncT.cs | 99 ++------------- .../Utils/Pipeline/BridgeComponentBase.cs | 2 +- .../Utils/Pipeline/PipelineComponent.cs | 22 ++-- src/Polly.Core/Utils/TaskHelper.cs | 16 --- src/Polly.Extensions/Telemetry/TagsList.cs | 30 ++--- .../HedgingExecutionContextTests.cs | 7 +- .../Hedging/Controller/TaskExecutionTests.cs | 27 +++- .../Polly.Core.Tests/Utils/TaskHelperTests.cs | 14 --- 9 files changed, 101 insertions(+), 235 deletions(-) diff --git a/src/Polly.Core/ResiliencePipeline.Sync.cs b/src/Polly.Core/ResiliencePipeline.Sync.cs index 724276270e9..4c027c184b9 100644 --- a/src/Polly.Core/ResiliencePipeline.Sync.cs +++ b/src/Polly.Core/ResiliencePipeline.Sync.cs @@ -13,32 +13,15 @@ public partial class ResiliencePipeline /// The context associated with the callback. /// The state associated with the callback. /// Thrown when or is . - public void Execute( - Action callback, - ResilienceContext context, - TState state) - { - Guard.NotNull(callback); - Guard.NotNull(context); - - InitializeSyncContext(context); - - Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => + public void Execute(Action callback, ResilienceContext context, TState state) + => Execute( + static (context, state) => { - try - { - state.callback(context, state.state); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state.callback(context, state.state); + return VoidResult.Instance; }, context, - (callback, state)).GetResultOrRethrow(); - } + (callback: Guard.NotNull(callback), state)); /// /// Executes the specified callback. @@ -46,31 +29,15 @@ [DebuggerDisableUserUnhandledExceptions] static (context, state) => /// The user-provided callback. /// The context associated with the callback. /// Thrown when or is . - public void Execute( - Action callback, - ResilienceContext context) - { - Guard.NotNull(callback); - Guard.NotNull(context); - - InitializeSyncContext(context); - - Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => + public void Execute(Action callback, ResilienceContext context) + => Execute( + static (context, state) => { - try - { - state(context); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state(context); + return VoidResult.Instance; }, context, - callback).GetResultOrRethrow(); - } + Guard.NotNull(callback)); /// /// Executes the specified callback. @@ -92,20 +59,13 @@ public void Execute( try { Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => + static (context, state) => { - try - { - state.callback(state.state, context.CancellationToken); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state.callback(state.state, context.CancellationToken); + return VoidResult.Instance; }, context, - (callback, state)).GetResultOrRethrow(); + (callback, state)); } finally { @@ -130,20 +90,13 @@ public void Execute( try { Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => + static (context, state) => { - try - { - state(context.CancellationToken); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state(context.CancellationToken); + return VoidResult.Instance; }, context, - callback).GetResultOrRethrow(); + callback); } finally { @@ -169,20 +122,13 @@ public void Execute( try { Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (_, state) => + static (_, state) => { - try - { - state.callback(state.state); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state.callback(state.state); + return VoidResult.Instance; }, context, - (callback, state)).GetResultOrRethrow(); + (callback, state)); } finally { @@ -204,20 +150,13 @@ public void Execute(Action callback) try { Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (_, state) => + static (_, state) => { - try - { - state(); - return Outcome.Void; - } - catch (Exception e) - { - return Outcome.FromException(e); - } + state(); + return VoidResult.Instance; }, context, - callback).GetResultOrRethrow(); + callback); } finally { @@ -226,6 +165,4 @@ [DebuggerDisableUserUnhandledExceptions] static (_, state) => } private ResilienceContext GetSyncContext(CancellationToken cancellationToken) => GetSyncContext(cancellationToken); - - private void InitializeSyncContext(ResilienceContext context) => InitializeSyncContext(context); } diff --git a/src/Polly.Core/ResiliencePipeline.SyncT.cs b/src/Polly.Core/ResiliencePipeline.SyncT.cs index da748e56d75..34a6b85b52f 100644 --- a/src/Polly.Core/ResiliencePipeline.SyncT.cs +++ b/src/Polly.Core/ResiliencePipeline.SyncT.cs @@ -25,21 +25,7 @@ public TResult Execute( InitializeSyncContext(context); - return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => - { - try - { - var result = state.callback(context, state.state); - return Outcome.FromResult(result); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, - context, - (callback, state)).GetResultOrRethrow(); + return Component.ExecuteCoreSync(callback, context, state); } /// @@ -50,31 +36,8 @@ [DebuggerDisableUserUnhandledExceptions] static (context, state) => /// The context associated with the callback. /// An instance of that represents the asynchronous execution. /// Thrown when or is . - public TResult Execute( - Func callback, - ResilienceContext context) - { - Guard.NotNull(callback); - Guard.NotNull(context); - - InitializeSyncContext(context); - - return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => - { - try - { - var result = state(context); - return Outcome.FromResult(result); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, - context, - callback).GetResultOrRethrow(); - } + public TResult Execute(Func callback, ResilienceContext context) + => Execute(static (context, state) => state(context), context, Guard.NotNull(callback)); /// /// Executes the specified callback. @@ -95,19 +58,9 @@ public TResult Execute( try { return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => - { - try - { - return Outcome.FromResult(state(context.CancellationToken)); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, + static (context, state) => state(context.CancellationToken), context, - callback).GetResultOrRethrow(); + callback); } finally { @@ -131,19 +84,9 @@ public TResult Execute(Func callback) try { return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (_, state) => - { - try - { - return Outcome.FromResult(state()); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, + static (_, state) => state(), context, - callback).GetResultOrRethrow(); + callback); } finally { @@ -169,19 +112,9 @@ public TResult Execute(Func callback, TState s try { return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (_, state) => - { - try - { - return Outcome.FromResult(state.callback(state.state)); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, + static (_, state) => state.callback(state.state), context, - (callback, state)).GetResultOrRethrow(); + (callback, state)); } finally { @@ -211,19 +144,9 @@ public TResult Execute( try { return Component.ExecuteCoreSync( - [DebuggerDisableUserUnhandledExceptions] static (context, state) => - { - try - { - return Outcome.FromResult(state.callback(state.state, context.CancellationToken)); - } - catch (Exception e) - { - return Outcome.FromException(e); - } - }, + static (context, state) => state.callback(state.state, context.CancellationToken), context, - (callback, state)).GetResultOrRethrow(); + (callback, state)); } finally { diff --git a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs index 5b3e1fa09e7..e90e94871c8 100644 --- a/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs +++ b/src/Polly.Core/Utils/Pipeline/BridgeComponentBase.cs @@ -27,6 +27,6 @@ protected static Outcome ConvertOutcome(Outcome outcome) return new(outcome.ExceptionDispatchInfo); } - return outcome.Result is null ? default : new((TTo)(object)outcome.Result); + return new((TTo?)(object?)outcome.Result); } } diff --git a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs index 8dd6848e404..319e9048e83 100644 --- a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs @@ -17,15 +17,23 @@ internal abstract ValueTask> ExecuteCore( ResilienceContext context, TState state); - internal Outcome ExecuteCoreSync( - Func> callback, + internal TResult ExecuteCoreSync( + Func callback, ResilienceContext context, TState state) - => ExecuteCore( - static (context, state) => new ValueTask>(state.callbackMethod(context, state.stateObject)), - context, - (callbackMethod: callback, stateObject: state)) - .GetResult(); + => ExecuteCore([DebuggerDisableUserUnhandledExceptions] static (context, state) => + { + try + { + return new ValueTask>(new Outcome(state.callback(context, state.state))); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception e) + { + return new ValueTask>(new Outcome(e)); + } + }, + context, (callback, state)).GetResult().GetResultOrRethrow(); public abstract ValueTask DisposeAsync(); diff --git a/src/Polly.Core/Utils/TaskHelper.cs b/src/Polly.Core/Utils/TaskHelper.cs index d874af21aba..2187ab2b979 100644 --- a/src/Polly.Core/Utils/TaskHelper.cs +++ b/src/Polly.Core/Utils/TaskHelper.cs @@ -5,22 +5,6 @@ internal static class TaskHelper { - public static void GetResult(this ValueTask task) - { - Debug.Assert( - task.IsCompleted, - "The value task should be already completed at this point. If not, it's an indication that the strategy does not respect the ResilienceContext.IsSynchronous value."); - - // Stryker disable once boolean : no means to test this - if (task.IsCompleted) - { - _ = task.Result; - return; - } - - task.Preserve().GetAwaiter().GetResult(); - } - public static TResult GetResult(this ValueTask task) { Debug.Assert( diff --git a/src/Polly.Extensions/Telemetry/TagsList.cs b/src/Polly.Extensions/Telemetry/TagsList.cs index 1fb2452582b..5c57f7dd008 100644 --- a/src/Polly.Extensions/Telemetry/TagsList.cs +++ b/src/Polly.Extensions/Telemetry/TagsList.cs @@ -1,33 +1,29 @@ +#if NET +using System.Runtime.InteropServices; +#endif using Polly.Utils; namespace Polly.Telemetry; internal sealed class TagsList { - private const int InitialArraySize = 20; + private static readonly ObjectPool ContextPool = new(static () => new TagsList(), static _ => true); - private static readonly ObjectPool ContextPool = new(static () => new TagsList(), static context => - { - context.Tags.Clear(); - return true; - }); - - private KeyValuePair[] _tagsArray = new KeyValuePair[InitialArraySize]; +#if !NET + private KeyValuePair[] _tagsArray = new KeyValuePair[20]; +#endif private TagsList() { } - internal static TagsList Get() - { - var context = ContextPool.Get(); - - return context; - } + internal static TagsList Get() => ContextPool.Get(); internal static void Return(TagsList context) { +#if !NET Array.Clear(context._tagsArray, 0, context.Tags.Count); +#endif context.Tags.Clear(); ContextPool.Return(context); } @@ -35,12 +31,15 @@ internal static void Return(TagsList context) /// /// Gets the tags associated with the resilience event. /// - public IList> Tags { get; } = []; + public List> Tags { get; } = []; internal ReadOnlySpan> TagsSpan { get { +#if NET + return CollectionsMarshal.AsSpan(Tags); +#else // stryker disable once equality : no means to test this if (Tags.Count > _tagsArray.Length) { @@ -53,6 +52,7 @@ internal static void Return(TagsList context) } return _tagsArray.AsSpan(0, Tags.Count); +#endif } } } diff --git a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs index 4407b53e5fc..d1b6b9d1f70 100644 --- a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs +++ b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs @@ -87,9 +87,12 @@ public async Task TryWaitForCompletedExecutionAsync_Initialized_Ok(int delay) (await task).ShouldBeNull(); } - [Fact] - public async Task TryWaitForCompletedExecutionAsync_FinishedTask_Ok() + [InlineData(false)] + [InlineData(true)] + [Theory] + public async Task TryWaitForCompletedExecutionAsync_FinishedTask_Ok(bool continueOnCapturedContext) { + _resilienceContext.ContinueOnCapturedContext = continueOnCapturedContext; var context = Create(); context.Initialize(_resilienceContext); await context.LoadExecutionAsync((_, _) => Outcome.FromResultAsValueTask(new DisposableResult("dummy")), "state"); diff --git a/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs b/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs index 74941059ede..7204e4f24f6 100644 --- a/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs +++ b/test/Polly.Core.Tests/Hedging/Controller/TaskExecutionTests.cs @@ -10,11 +10,11 @@ public class TaskExecutionTests : IDisposable { private const string Handled = "Handled"; private readonly ResiliencePropertyKey _myKey = new("my-key"); - private readonly HedgingHandler _hedgingHandler; private readonly CancellationTokenSource _cts; private readonly HedgingTimeProvider _timeProvider; private readonly ResilienceStrategyTelemetry _telemetry; private readonly List _args = []; + private HedgingHandler _hedgingHandler; private ResilienceContext _primaryContext; public TaskExecutionTests() @@ -104,6 +104,31 @@ public async Task Initialize_Secondary_Ok(string value, bool handled) AssertContext(execution.Context); } + [InlineData(false)] + [InlineData(true)] + [Theory] + public async Task Initialize_Secondary_DefaultHandler_Ok(bool sync) + { + _hedgingHandler = HedgingHelper.CreateHandler(_ => false, new HedgingStrategyOptions().ActionGenerator); + _primaryContext.Initialize(sync); + var execution = Create(); + + Func>> primaryCallback = (context, state) => + { + AssertContext(context); + state.ShouldBe("dummy-state"); + return Outcome.FromResultAsValueTask(new DisposableResult { Name = "Unhandled" }); + }; + + (await execution.InitializeAsync(HedgedTaskType.Secondary, _primaryContext, primaryCallback, "dummy-state", 4)).ShouldBeTrue(); + + await execution.ExecutionTaskSafe!; + + execution.Outcome.Result!.Name.ShouldBe("Unhandled"); + execution.IsHandled.ShouldBe(false); + AssertContext(execution.Context); + } + [Fact] public async Task Initialize_SecondaryWhenTaskGeneratorReturnsNull_Ok() { diff --git a/test/Polly.Core.Tests/Utils/TaskHelperTests.cs b/test/Polly.Core.Tests/Utils/TaskHelperTests.cs index 8436f3e2a47..bdccc186048 100644 --- a/test/Polly.Core.Tests/Utils/TaskHelperTests.cs +++ b/test/Polly.Core.Tests/Utils/TaskHelperTests.cs @@ -22,18 +22,4 @@ static async ValueTask GetValue() return 42; } } - - [Fact] - public void GetResult_ValueTask_Ok() - { - TaskHelper.GetResult(default); - - Should.NotThrow(() => TaskHelper.GetResult(GetValue())); - - static async ValueTask GetValue() - { - await Task.Delay(20); - return VoidResult.Instance; - } - } } From ff0cf20c777daa28e49f2f81b00c33dbf71b76be Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Mon, 18 Aug 2025 16:34:22 +0300 Subject: [PATCH 04/10] More cleanup and test coverage. --- .../CircuitBreakerResilienceStrategy.cs | 13 ++-------- .../Fallback/FallbackResilienceStrategy.cs | 11 +------- .../Hedging/Controller/HedgingController.cs | 6 ++--- .../Retry/RetryResilienceStrategy.cs | 13 +--------- .../Simmy/Behavior/ChaosBehaviorStrategy.cs | 13 ++-------- .../Simmy/Fault/ChaosFaultStrategy.cs | 13 ++-------- .../Simmy/Latency/ChaosLatencyStrategy.cs | 13 ++-------- .../Simmy/Outcomes/ChaosOutcomeStrategy.cs | 13 ++-------- src/Polly.Extensions/Telemetry/TagsList.cs | 25 ++++++------------- .../Telemetry/TelemetryListenerImpl.cs | 6 ++--- .../HedgingExecutionContextTests.cs | 14 +++++------ 11 files changed, 33 insertions(+), 107 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index 2eeb24f5699..1c6bf68e016 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -34,17 +34,8 @@ protected internal override async ValueTask> ExecuteCore(Func return outcome; } - try - { - context.CancellationToken.ThrowIfCancellationRequested(); - outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); - } -#pragma warning disable CA1031 - catch (Exception ex) - { - outcome = new(ex); - } -#pragma warning restore CA1031 + context.CancellationToken.ThrowIfCancellationRequested(); + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); var args = new CircuitBreakerPredicateArguments(context, outcome); if (await _handler(args).ConfigureAwait(context.ContinueOnCapturedContext)) diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs index 4fed52e0452..fae90b4837a 100644 --- a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs +++ b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs @@ -19,16 +19,7 @@ public FallbackResilienceStrategy(FallbackHandler handler, Func> ExecuteCore(Func>> callback, ResilienceContext context, TState state) { - Outcome outcome; - try - { - outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); - } - catch (Exception ex) - { - outcome = new(ex); - } - + var outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); var handleFallbackArgs = new FallbackPredicateArguments(context, outcome); if (!await _handler.ShouldHandle(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext)) { diff --git a/src/Polly.Core/Hedging/Controller/HedgingController.cs b/src/Polly.Core/Hedging/Controller/HedgingController.cs index 1cd7cc4e14e..9e998734868 100644 --- a/src/Polly.Core/Hedging/Controller/HedgingController.cs +++ b/src/Polly.Core/Hedging/Controller/HedgingController.cs @@ -32,11 +32,12 @@ public HedgingController( return true; }); + Action> onReset = null!; _contextPool = new ObjectPool>( () => { Interlocked.Increment(ref _rentedContexts); - return new HedgingExecutionContext(_executionPool, provider, maxAttempts, ReturnContext); + return new HedgingExecutionContext(_executionPool, provider, maxAttempts, onReset); }, _ => { @@ -45,6 +46,7 @@ public HedgingController( // Stryker disable once Boolean : no means to test this return true; }); + onReset = _contextPool.Return; } public int RentedContexts => _rentedContexts; @@ -57,6 +59,4 @@ public HedgingExecutionContext GetContext(ResilienceContext context) executionContext.Initialize(context); return executionContext; } - - private void ReturnContext(HedgingExecutionContext context) => _contextPool.Return(context); } diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs index c4b78fcfb49..56b43375357 100644 --- a/src/Polly.Core/Retry/RetryResilienceStrategy.cs +++ b/src/Polly.Core/Retry/RetryResilienceStrategy.cs @@ -52,18 +52,7 @@ protected internal override async ValueTask> ExecuteCore(Func while (true) { var startTimestamp = _timeProvider.GetTimestamp(); - Outcome outcome; - try - { - outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); - } -#pragma warning disable CA1031 - catch (Exception ex) - { - outcome = new(ex); - } -#pragma warning restore CA1031 - + var outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); var shouldRetryArgs = new RetryPredicateArguments(context, outcome, attempt); var handle = await ShouldHandle(shouldRetryArgs).ConfigureAwait(context.ContinueOnCapturedContext); var executionTime = _timeProvider.GetElapsedTime(startTimestamp); diff --git a/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs b/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs index 9c47110f105..7cc33736d09 100644 --- a/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs +++ b/src/Polly.Core/Simmy/Behavior/ChaosBehaviorStrategy.cs @@ -40,17 +40,8 @@ protected internal override async ValueTask> ExecuteCore> ExecuteCore> ExecuteCore> { private static readonly ObjectPool ContextPool = new(static () => new TagsList(), static _ => true); @@ -22,36 +22,27 @@ private TagsList() internal static void Return(TagsList context) { #if !NET - Array.Clear(context._tagsArray, 0, context.Tags.Count); + Array.Clear(context._tagsArray, 0, context.Count); #endif - context.Tags.Clear(); + context.Clear(); ContextPool.Return(context); } - /// - /// Gets the tags associated with the resilience event. - /// - public List> Tags { get; } = []; - internal ReadOnlySpan> TagsSpan { get { #if NET - return CollectionsMarshal.AsSpan(Tags); + return CollectionsMarshal.AsSpan(this); #else // stryker disable once equality : no means to test this - if (Tags.Count > _tagsArray.Length) - { - Array.Resize(ref _tagsArray, Tags.Count); - } - - for (int i = 0; i < Tags.Count; i++) + if (Count > _tagsArray.Length) { - _tagsArray[i] = Tags[i]; + Array.Resize(ref _tagsArray, Count); } - return _tagsArray.AsSpan(0, Tags.Count); + CopyTo(_tagsArray, 0); + return _tagsArray.AsSpan(0, Count); #endif } } diff --git a/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs b/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs index 90a87855ae3..b2f7ded058f 100644 --- a/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs +++ b/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs @@ -120,7 +120,7 @@ private void MeterEvent(in TelemetryEventArguments(in args, tags.Tags); + var context = new EnrichmentContext(in args, tags); UpdateEnrichmentContext(in context, severity); ExecutionDuration.Record(executionFinished.Duration.TotalMilliseconds, tags.TagsSpan); TagsList.Return(tags); @@ -131,7 +131,7 @@ private void MeterEvent(in TelemetryEventArguments(in args, tags.Tags); + var context = new EnrichmentContext(in args, tags); UpdateEnrichmentContext(in context, severity); context.Tags.Add(new(ResilienceTelemetryTags.AttemptNumber, executionAttempt.AttemptNumber.AsBoxedInt())); context.Tags.Add(new(ResilienceTelemetryTags.AttemptHandled, executionAttempt.Handled.AsBoxedBool())); @@ -142,7 +142,7 @@ private void MeterEvent(in TelemetryEventArguments(in args, tags.Tags); + var context = new EnrichmentContext(in args, tags); UpdateEnrichmentContext(in context, severity); Counter.Add(1, tags.TagsSpan); TagsList.Return(tags); diff --git a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs index d1b6b9d1f70..a324dbf0c9d 100644 --- a/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs +++ b/test/Polly.Core.Tests/Hedging/Controller/HedgingExecutionContextTests.cs @@ -87,12 +87,9 @@ public async Task TryWaitForCompletedExecutionAsync_Initialized_Ok(int delay) (await task).ShouldBeNull(); } - [InlineData(false)] - [InlineData(true)] - [Theory] - public async Task TryWaitForCompletedExecutionAsync_FinishedTask_Ok(bool continueOnCapturedContext) + [Fact] + public async Task TryWaitForCompletedExecutionAsync_FinishedTask_Ok() { - _resilienceContext.ContinueOnCapturedContext = continueOnCapturedContext; var context = Create(); context.Initialize(_resilienceContext); await context.LoadExecutionAsync((_, _) => Outcome.FromResultAsValueTask(new DisposableResult("dummy")), "state"); @@ -151,9 +148,12 @@ public async Task TryWaitForCompletedExecutionAsync_SynchronousExecution_Ok() context.Tasks[0].AcceptOutcome(); } - [Fact] - public async Task TryWaitForCompletedExecutionAsync_HedgedExecution_Ok() + [InlineData(false)] + [InlineData(true)] + [Theory] + public async Task TryWaitForCompletedExecutionAsync_HedgedExecution_Ok(bool continueOnCapturedContext) { + _resilienceContext.ContinueOnCapturedContext = continueOnCapturedContext; var context = Create(); context.Initialize(_resilienceContext); ConfigureSecondaryTasks(TimeSpan.FromHours(1), TimeSpan.FromHours(1)); From 532fb4015bebfd83308b01b4b956ff0a0fb57f95 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Tue, 19 Aug 2025 15:11:45 +0300 Subject: [PATCH 05/10] More cleanup and test coverage. --- .../CircuitBreakerResilienceStrategy.cs | 13 +++- .../Fallback/FallbackResilienceStrategy.cs | 11 ++- .../Hedging/Controller/TaskExecution.cs | 25 +++--- .../Retry/RetryResilienceStrategy.cs | 13 +++- .../Timeout/TimeoutResilienceStrategy.cs | 11 ++- .../Utils/Pipeline/BridgeComponent.TResult.cs | 8 +- .../Utils/Pipeline/ReloadableComponent.cs | 77 +++++++++---------- .../CircuitBreakerResilienceStrategyTests.cs | 12 +++ .../FallbackResilienceStrategyTests.cs | 17 ++++ .../Retry/RetryResilienceStrategyTests.cs | 8 ++ 10 files changed, 126 insertions(+), 69 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index 1c6bf68e016..2eeb24f5699 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -34,8 +34,17 @@ protected internal override async ValueTask> ExecuteCore(Func return outcome; } - context.CancellationToken.ThrowIfCancellationRequested(); - outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + outcome = new(ex); + } +#pragma warning restore CA1031 var args = new CircuitBreakerPredicateArguments(context, outcome); if (await _handler(args).ConfigureAwait(context.ContinueOnCapturedContext)) diff --git a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs index fae90b4837a..4fed52e0452 100644 --- a/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs +++ b/src/Polly.Core/Fallback/FallbackResilienceStrategy.cs @@ -19,7 +19,16 @@ public FallbackResilienceStrategy(FallbackHandler handler, Func> ExecuteCore(Func>> callback, ResilienceContext context, TState state) { - var outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + Outcome outcome; + try + { + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } + catch (Exception ex) + { + outcome = new(ex); + } + var handleFallbackArgs = new FallbackPredicateArguments(context, outcome); if (!await _handler.ShouldHandle(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext)) { diff --git a/src/Polly.Core/Hedging/Controller/TaskExecution.cs b/src/Polly.Core/Hedging/Controller/TaskExecution.cs index d7e7ee6e0cb..29bc8ab430f 100644 --- a/src/Polly.Core/Hedging/Controller/TaskExecution.cs +++ b/src/Polly.Core/Hedging/Controller/TaskExecution.cs @@ -26,7 +26,7 @@ internal sealed class TaskExecution private readonly ResilienceStrategyTelemetry _telemetry; private readonly HedgingHandler _handler; private CancellationTokenSource? _cancellationSource; - private CancellationTokenRegistration? _cancellationRegistration; + private CancellationTokenRegistration _cancellationRegistration; private ResilienceContext? _activeContext; private long _startExecutionTimestamp; private long _stopExecutionTimestamp; @@ -99,10 +99,11 @@ public async ValueTask InitializeAsync( _activeContext = _cachedContext; _activeContext.InitializeFrom(primaryContext, _cancellationSource!.Token); - if (primaryContext.CancellationToken.CanBeCanceled) - { - _cancellationRegistration = primaryContext.CancellationToken.Register(o => ((CancellationTokenSource)o!).Cancel(), _cancellationSource); - } +#if NET + _cancellationRegistration = primaryContext.CancellationToken.UnsafeRegister(static o => ((CancellationTokenSource)o!).Cancel(), _cancellationSource); +#else + _cancellationRegistration = primaryContext.CancellationToken.Register(static o => ((CancellationTokenSource)o!).Cancel(), _cancellationSource); +#endif if (type == HedgedTaskType.Secondary) { @@ -154,16 +155,10 @@ public async ValueTask ResetAsync() { OnReset?.Invoke(this); - if (_cancellationRegistration is { } registration) - { -#if NETCOREAPP - await registration.DisposeAsync().ConfigureAwait(false); -#else - registration.Dispose(); -#endif - } - - _cancellationRegistration = null; +#pragma warning disable CA1849, S6966 // Call async methods when in an async method, OK here as the callback is synchronous + _cancellationRegistration.Dispose(); + _cancellationRegistration = default; +#pragma warning restore CA1849, S6966 if (!IsAccepted) { diff --git a/src/Polly.Core/Retry/RetryResilienceStrategy.cs b/src/Polly.Core/Retry/RetryResilienceStrategy.cs index 56b43375357..c4b78fcfb49 100644 --- a/src/Polly.Core/Retry/RetryResilienceStrategy.cs +++ b/src/Polly.Core/Retry/RetryResilienceStrategy.cs @@ -52,7 +52,18 @@ protected internal override async ValueTask> ExecuteCore(Func while (true) { var startTimestamp = _timeProvider.GetTimestamp(); - var outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + Outcome outcome; + try + { + outcome = await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); + } +#pragma warning disable CA1031 + catch (Exception ex) + { + outcome = new(ex); + } +#pragma warning restore CA1031 + var shouldRetryArgs = new RetryPredicateArguments(context, outcome, attempt); var handle = await ShouldHandle(shouldRetryArgs).ConfigureAwait(context.ContinueOnCapturedContext); var executionTime = _timeProvider.GetElapsedTime(startTimestamp); diff --git a/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs b/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs index 53146ab1803..318f229f223 100644 --- a/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs +++ b/src/Polly.Core/Timeout/TimeoutResilienceStrategy.cs @@ -94,11 +94,10 @@ protected internal override async ValueTask> ExecuteCore ((CancellationTokenSource)state!).Cancel(), cancellationSource, useSynchronizationContext: false); - } - - return default; +#if NET + return previousToken.UnsafeRegister(static state => ((CancellationTokenSource)state!).Cancel(), cancellationSource); +#else + return previousToken.Register(static state => ((CancellationTokenSource)state!).Cancel(), cancellationSource); +#endif } } diff --git a/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs b/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs index 2e9ae5336a2..80b058ae507 100644 --- a/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs +++ b/src/Polly.Core/Utils/Pipeline/BridgeComponent.TResult.cs @@ -18,9 +18,11 @@ internal override ValueTask> ExecuteCore( // Check if we can cast directly, thus saving some cycles and improving the performance if (callback is Func>> casted) { -#pragma warning disable CA2012 - return (ValueTask>)(object)Strategy.ExecuteCore(casted, context, state); -#pragma warning restore CA2012 + var task = Strategy.ExecuteCore(casted, context, state); + + // Using Unsafe.As avoids boxing allocations that would occur with a cast through object. + Debug.Assert(task is ValueTask>, "Callback return type is identical to strategy return type"); + return Unsafe.As>, ValueTask>>(ref task); } else { diff --git a/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs b/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs index 91c438ce2e7..96cb1047792 100644 --- a/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/ReloadableComponent.cs @@ -14,19 +14,16 @@ internal sealed class ReloadableComponent : PipelineComponent private readonly Func _factory; private ResilienceStrategyTelemetry _telemetry; - private CancellationTokenSource _tokenSource = null!; - private CancellationTokenRegistration _registration; - private List _reloadTokens; + private CancellationTokenSource? _tokenSource; public ReloadableComponent(Entry entry, Func factory) { Component = entry.Component; - _reloadTokens = entry.ReloadTokens; _factory = factory; _telemetry = entry.Telemetry; - TryRegisterOnReload(); + TryRegisterOnReload(entry.ReloadTokens); } public PipelineComponent Component { get; private set; } @@ -38,64 +35,62 @@ internal override ValueTask> ExecuteCore( public override ValueTask DisposeAsync() { - DisposeRegistration(); + _tokenSource?.Dispose(); return Component.DisposeAsync(); } - private void TryRegisterOnReload() + private void TryRegisterOnReload(List reloadTokens) { - if (_reloadTokens.Count == 0) + if (reloadTokens.Count == 0) { return; } -#pragma warning disable S3878 // Arrays should not be created for params parameters - _tokenSource = CancellationTokenSource.CreateLinkedTokenSource([.. _reloadTokens]); -#pragma warning restore S3878 // Arrays should not be created for params parameters - _registration = _tokenSource.Token.Register(() => - { - var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); - var previousComponent = Component; - - try - { - _telemetry.Report(new(ResilienceEventSeverity.Information, OnReloadEvent), context, new OnReloadArguments()); - (Component, _reloadTokens, _telemetry) = _factory(); - } - catch (Exception e) - { - _reloadTokens = []; - _telemetry.Report(new(ResilienceEventSeverity.Error, ReloadFailedEvent), context, Outcome.FromException(e), new ReloadFailedArguments(e)); - ResilienceContextPool.Shared.Return(context); - } - - DisposeRegistration(); - TryRegisterOnReload(); - - _ = DisposeDiscardedComponentSafeAsync(previousComponent); - }); + _tokenSource = CancellationTokenSource.CreateLinkedTokenSource([.. reloadTokens]); +#if NET + _ = _tokenSource.Token.UnsafeRegister(static s => ((ReloadableComponent)s!).Reload(), this); +#else + _ = _tokenSource.Token.Register(static s => ((ReloadableComponent)s!).Reload(), this); +#endif } - private async Task DisposeDiscardedComponentSafeAsync(PipelineComponent component) + private void Reload() { - var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: false); + _tokenSource!.Dispose(); + _tokenSource = null; + var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); + _telemetry.Report(new(ResilienceEventSeverity.Information, OnReloadEvent), context, new OnReloadArguments()); + ResilienceContextPool.Shared.Return(context); + + var previousComponent = Component; + List reloadTokens; try { - await component.DisposeAsync().ConfigureAwait(false); + (Component, reloadTokens, _telemetry) = _factory(); } catch (Exception e) { - _telemetry.Report(new(ResilienceEventSeverity.Error, DisposeFailedEvent), context, Outcome.FromException(e), new DisposedFailedArguments(e)); + context = new ResilienceContext().Initialize(isSynchronous: true); + _telemetry.Report(new(ResilienceEventSeverity.Error, ReloadFailedEvent), context, Outcome.FromException(e), new ReloadFailedArguments(e)); + return; } - ResilienceContextPool.Shared.Return(context); + TryRegisterOnReload(reloadTokens); + _ = DisposeDiscardedComponentSafeAsync(previousComponent); } - private void DisposeRegistration() + private async Task DisposeDiscardedComponentSafeAsync(PipelineComponent component) { - _registration.Dispose(); - _tokenSource.Dispose(); + try + { + await component.DisposeAsync().ConfigureAwait(false); + } + catch (Exception e) + { + var context = new ResilienceContext().Initialize(isSynchronous: false); + _telemetry.Report(new(ResilienceEventSeverity.Error, DisposeFailedEvent), context, Outcome.FromException(e), new DisposedFailedArguments(e)); + } } internal sealed record ReloadFailedArguments(Exception Exception); diff --git a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs index 07ac5e3b9b3..42c412b48ec 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyTests.cs @@ -117,6 +117,18 @@ public void Execute_UnhandledException_OnActionSuccess() _behavior.Received(1).OnActionSuccess(CircuitState.Closed); } + [Fact] + public async Task ExecuteOutcomeAsync_UnhandledException_OnActionSuccess() + { + _options.ShouldHandle = args => new ValueTask(args.Outcome.Exception is InvalidOperationException); + var strategy = Create(); + + var outcome = await strategy.ExecuteOutcomeAsync((_, _) => throw new ArgumentException(), new(), "dummy-state"); + outcome.Exception.ShouldBeOfType(); + + _behavior.Received(1).OnActionSuccess(CircuitState.Closed); + } + public void Dispose() => _controller.Dispose(); [Fact] diff --git a/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs b/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs index 30076959cab..a3890a2b8fc 100644 --- a/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/Fallback/FallbackResilienceStrategyTests.cs @@ -118,6 +118,23 @@ public void Handle_UnhandledResult_Ok() fallbackActionCalled.ShouldBeFalse(); } + [Fact] + public async Task ExecuteOutcomeAsync_UnhandledException_Ok() + { + var called = false; + var fallbackActionCalled = false; + + _options.OnFallback = _ => { called = true; return default; }; + SetHandler(outcome => outcome.Exception is InvalidOperationException, () => { fallbackActionCalled = true; return Outcome.FromResult("secondary"); }); + + var outcome = await Create().ExecuteOutcomeAsync((_, _) => throw new ArgumentException(), new(), "dummy-state"); + outcome.Exception.ShouldBeOfType(); + + _args.ShouldBeEmpty(); + called.ShouldBeFalse(); + fallbackActionCalled.ShouldBeFalse(); + } + private void SetHandler( Func, bool> shouldHandle, Func> fallback) => diff --git a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs index 8cae859abf9..c1dfdb11464 100644 --- a/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs +++ b/test/Polly.Core.Tests/Retry/RetryResilienceStrategyTests.cs @@ -69,6 +69,14 @@ public async Task ExecuteAsync_CanceledDuringExecution_EnsureResultReturned() executions.ShouldBe(1); } + [Fact] + public async Task ExecuteAsync_ExceptionInExecution_EnsureResultReturned() + { + var sut = CreateSut(); + var result = await sut.ExecuteOutcomeAsync((_, _) => throw new ArgumentException(), ResilienceContextPool.Shared.Get(), default); + result.Exception.ShouldBeOfType(); + } + [Fact] public async Task ExecuteAsync_CanceledDuringExecution_EnsureNotExecutedAgain() { From 0882158b948e6f955005e7c7bbb1d9447f482080 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Thu, 21 Aug 2025 15:37:09 +0300 Subject: [PATCH 06/10] Address PR feedback --- .../Controller/CircuitStateController.cs | 71 ++++++++----------- .../Utils/Pipeline/PipelineComponent.cs | 1 + .../PollyServiceCollectionExtensions.cs | 2 +- .../Controller/ScheduledTaskExecutorTests.cs | 8 ++- .../Pipeline/PipelineComponentFactoryTests.cs | 4 +- 5 files changed, 42 insertions(+), 44 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs index 3173c64b7f8..ad30dc4f55e 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs @@ -91,7 +91,7 @@ public Task IsolateCircuitAsync(ResilienceContext context) context.Initialize(isSynchronous: false); - Task? task; + Task task; lock (_lock) { @@ -111,7 +111,7 @@ public Task CloseCircuitAsync(ResilienceContext context) context.Initialize(isSynchronous: false); - Task? task; + Task task; lock (_lock) { @@ -121,14 +121,14 @@ public Task CloseCircuitAsync(ResilienceContext context) return ExecuteScheduledTaskAsync(task, context); } - public async ValueTask?> OnActionPreExecuteAsync(ResilienceContext context) + public ValueTask?> OnActionPreExecuteAsync(ResilienceContext context) { EnsureNotDisposed(); BrokenCircuitException? exception = null; bool isHalfOpen = false; - Task? task = null; + var task = Task.CompletedTask; lock (_lock) { @@ -155,14 +155,27 @@ public Task CloseCircuitAsync(ResilienceContext context) } } - await ExecuteScheduledTaskAsync(task, context).ConfigureAwait(context.ContinueOnCapturedContext); - if (exception is not null) { _telemetry.SetTelemetrySource(exception); - return Outcome.FromException(exception); + return new(result: new(exception)); } + task = ExecuteScheduledTaskAsync(task, context); + if (!task.IsCompleted) + { + return WaitHalfOpenTask(task, context.ContinueOnCapturedContext); + } + +#pragma warning disable CA1849 // Call async methods when in an async method + task.GetAwaiter().GetResult(); +#pragma warning restore CA1849 + return default; + } + + private static async ValueTask?> WaitHalfOpenTask(Task task, bool continueOnCapturedContext) + { + await task.ConfigureAwait(continueOnCapturedContext); return null; } @@ -170,7 +183,7 @@ public Task OnUnhandledOutcomeAsync(Outcome outcome, ResilienceContext contex { EnsureNotDisposed(); - Task? task = null; + var task = Task.CompletedTask; lock (_lock) { @@ -196,7 +209,7 @@ public Task OnHandledOutcomeAsync(Outcome outcome, ResilienceContext context) { EnsureNotDisposed(); - Task? task = null; + var task = Task.CompletedTask; lock (_lock) { @@ -227,35 +240,17 @@ public void Dispose() _disposed = true; } - internal static Task ExecuteScheduledTaskAsync(Task? task, ResilienceContext context) + internal static Task ExecuteScheduledTaskAsync(Task task, ResilienceContext context) { - if (task is not null) + if (context.IsSynchronous && !task.IsCompleted) { - if (context.IsSynchronous && !task.IsCompleted) - { #pragma warning disable CA1849 // Call async methods when in an async method - // because this is synchronous execution we need to block -#if NET8_0_OR_GREATER - task.ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing).GetAwaiter().GetResult(); -#else - try - { - task.GetAwaiter().GetResult(); - } -#pragma warning disable CA1031 - catch - { - // exception will be observed by the awaiter of this method - } -#pragma warning restore CA1031 -#endif + // because this is synchronous execution we need to block + task.GetAwaiter().GetResult(); #pragma warning restore CA1849 // Call async methods when in an async method - } - - return task; } - return Task.CompletedTask; + return task; } private static bool IsDateTimeOverflow(DateTimeOffset utcNow, TimeSpan breakDuration) @@ -279,7 +274,7 @@ private void EnsureNotDisposed() } #endif - private Task? CloseCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceContext context) + private Task CloseCircuit_NeedsLock(Outcome outcome, bool manual, ResilienceContext context) { _blockedUntil = DateTimeOffset.MinValue; _lastOutcome = null; @@ -300,9 +295,7 @@ private void EnsureNotDisposed() } } -#pragma warning disable S4586 - return null; -#pragma warning restore S4586 + return Task.CompletedTask; } private bool PermitHalfOpenCircuitTest_NeedsLock() @@ -331,7 +324,7 @@ private BrokenCircuitException CreateBrokenCircuitException() return exception; } - private Task? OpenCircuitFor_NeedsLock(Outcome outcome, TimeSpan breakDuration, bool manual, ResilienceContext context) + private Task OpenCircuitFor_NeedsLock(Outcome outcome, TimeSpan breakDuration, bool manual, ResilienceContext context) { var utcNow = _timeProvider.GetUtcNow(); @@ -355,9 +348,7 @@ private BrokenCircuitException CreateBrokenCircuitException() return _executor.ScheduleTask(() => _onOpened(args).AsTask()); } -#pragma warning disable S4586 - return null; -#pragma warning restore S4586 + return Task.CompletedTask; } private Task ScheduleHalfOpenTask(ResilienceContext context) diff --git a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs index 319e9048e83..e06eb432e7c 100644 --- a/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs +++ b/src/Polly.Core/Utils/Pipeline/PipelineComponent.cs @@ -29,6 +29,7 @@ internal TResult ExecuteCoreSync( } #pragma warning disable CA1031 // Do not catch general exception types catch (Exception e) +#pragma warning restore CA1031 { return new ValueTask>(new Outcome(e)); } diff --git a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs index ad98e604eff..b8d6ac99de4 100644 --- a/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs +++ b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs @@ -258,7 +258,7 @@ public static IServiceCollection AddResiliencePipelineRegistry(this IServi .AddOptions>() .Configure((options, serviceProvider) => { - options.BuilderFactory = () => serviceProvider.GetRequiredService(); + options.BuilderFactory = serviceProvider.GetRequiredService; }); return services; diff --git a/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs b/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs index 87a18df798a..96ce209863b 100644 --- a/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs +++ b/test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs @@ -4,6 +4,8 @@ namespace Polly.Core.Tests.CircuitBreaker.Controller; public class ScheduledTaskExecutorTests { + private static CancellationToken CancellationToken => CancellationToken.None; + [Fact] public async Task ScheduleTask_Success_EnsureExecuted() { @@ -61,7 +63,7 @@ public async Task ScheduleTask_Multiple_EnsureExecutionSerialized() var otherTask = scheduler.ScheduleTask(() => Task.CompletedTask); #pragma warning disable xUnit1031 // Do not use blocking task operations in test method - otherTask.Wait(50).ShouldBeFalse(); + otherTask.Wait(50, CancellationToken).ShouldBeFalse(); #pragma warning restore xUnit1031 // Do not use blocking task operations in test method verified.Set(); @@ -119,7 +121,11 @@ public void Dispose_WhenScheduledTaskExecuting() disposed.Set(); #pragma warning disable xUnit1031 +#if NET + scheduler.ProcessingTask.Wait(timeout, CancellationToken).ShouldBeTrue(); +#else scheduler.ProcessingTask.Wait(timeout).ShouldBeTrue(); +#endif #pragma warning restore xUnit1031 } diff --git a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs index 9e849da60ff..3f934c83746 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs @@ -29,7 +29,7 @@ public class PipelineComponentFactoryTests public void WithDisposableCallbacks_NoCallbacks_ReturnsOriginalComponent(IEnumerable callbacks) { var component = Substitute.For(); - var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks.ToList()); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, [..callbacks]); result.ShouldBeSameAs(component); } @@ -41,7 +41,7 @@ public void PipelineComponentFactory_Should_Return_WrapperComponent_With_Callbac { var component = Substitute.For(); - var result = PipelineComponentFactory.WithDisposableCallbacks(component, callbacks.ToList()); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, [.. callbacks]); result.ShouldBeOfType(); } From 6631630b7f9b8a353e4579d6a3cd563e5866fa3a Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Thu, 21 Aug 2025 16:31:25 +0300 Subject: [PATCH 07/10] Fix formatting --- .../Utils/Pipeline/PipelineComponentFactoryTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs index 3f934c83746..40eb7f2b26d 100644 --- a/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs +++ b/test/Polly.Core.Tests/Utils/Pipeline/PipelineComponentFactoryTests.cs @@ -29,7 +29,7 @@ public class PipelineComponentFactoryTests public void WithDisposableCallbacks_NoCallbacks_ReturnsOriginalComponent(IEnumerable callbacks) { var component = Substitute.For(); - var result = PipelineComponentFactory.WithDisposableCallbacks(component, [..callbacks]); + var result = PipelineComponentFactory.WithDisposableCallbacks(component, [.. callbacks]); result.ShouldBeSameAs(component); } From 07e20014791a48344c4bc77627663fd0ff6db1c8 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Thu, 21 Aug 2025 17:34:25 +0300 Subject: [PATCH 08/10] Try to fix mutation score --- .../CircuitBreaker/Controller/CircuitStateController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs index ad30dc4f55e..8a2133265a3 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs @@ -168,6 +168,7 @@ public Task CloseCircuitAsync(ResilienceContext context) } #pragma warning disable CA1849 // Call async methods when in an async method + Debug.Assert(task.IsCompleted, "Async flow is handled separately"); task.GetAwaiter().GetResult(); #pragma warning restore CA1849 return default; From bba0ad76efbc69c06c4764a357cc84a46c00a9d5 Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Thu, 21 Aug 2025 19:29:22 +0300 Subject: [PATCH 09/10] Try to fix mutation score. Cleanup. --- .../CircuitBreakerManualControl.cs | 105 ++++++------------ .../Controller/HedgingExecutionContext.cs | 12 +- 2 files changed, 46 insertions(+), 71 deletions(-) diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs index 4eca9c8c355..282b9495a7a 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerManualControl.cs @@ -30,53 +30,30 @@ public CircuitBreakerManualControl() internal IDisposable Initialize(Func onIsolate, Func onReset) { + bool isolated; lock (_lock) { _onIsolate.Add(onIsolate); _onReset.Add(onReset); + isolated = _isolated; + } - 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); - } - }); + // if the control indicates that circuit breaker should be isolated, we isolate it right away + if (isolated) + { + var context = ResilienceContextPool.Shared.Get().Initialize(isSynchronous: true); + onIsolate(context).GetAwaiter().GetResult(); } + + return new RegistrationDisposable(this, onIsolate, onReset); } - /// - /// Isolates (opens) the circuit manually, and holds it in this state until a call to is made. - /// - /// The resilience context. - /// The instance of that represents the asynchronous execution. - /// Thrown when is . - /// Thrown when calling this method after this object is disposed. - internal async Task IsolateAsync(ResilienceContext context) + private void Remove(Func onIsolate, Func onReset) { - Guard.NotNull(context); - - _isolated = true; - - Func[] callbacks; - lock (_lock) { - callbacks = _onIsolate.ToArray(); - } - - foreach (var action in callbacks) - { - await action(context).ConfigureAwait(context.ContinueOnCapturedContext); + _onIsolate.Remove(onIsolate); + _onReset.Remove(onReset); } } @@ -88,11 +65,21 @@ internal async Task IsolateAsync(ResilienceContext context) /// Thrown when calling this method after this object is disposed. public async Task IsolateAsync(CancellationToken cancellationToken = default) { + Func[] callbacks; + lock (_lock) + { + callbacks = _onIsolate.ToArray(); + _isolated = true; + } + var context = ResilienceContextPool.Shared.Get(cancellationToken).Initialize(isSynchronous: false); try { - await IsolateAsync(context).ConfigureAwait(false); + foreach (var action in callbacks) + { + await action(context).ConfigureAwait(context.ContinueOnCapturedContext); + } } finally { @@ -103,44 +90,26 @@ public async Task IsolateAsync(CancellationToken cancellationToken = default) /// /// Closes the circuit, and resets any statistics controlling automated circuit-breaking. /// - /// The resilience context. + /// The cancellation token. /// The instance of that represents the asynchronous execution. - /// Thrown when is . /// Thrown when calling this method after this object is disposed. - internal async Task CloseAsync(ResilienceContext context) + public async Task CloseAsync(CancellationToken cancellationToken = default) { - Guard.NotNull(context); - - _isolated = false; - - context.Initialize(isSynchronous: false); - Func[] callbacks; - lock (_lock) { callbacks = _onReset.ToArray(); + _isolated = false; } - foreach (var action in callbacks) - { - await action(context).ConfigureAwait(context.ContinueOnCapturedContext); - } - } - - /// - /// Closes the circuit, and resets any statistics controlling automated circuit-breaking. - /// - /// The cancellation token. - /// The instance of that represents the asynchronous execution. - /// Thrown when calling this method after this object is disposed. - public async Task CloseAsync(CancellationToken cancellationToken = default) - { - var context = ResilienceContextPool.Shared.Get(cancellationToken); + var context = ResilienceContextPool.Shared.Get(cancellationToken).Initialize(isSynchronous: false); try { - await CloseAsync(context).ConfigureAwait(false); + foreach (var action in callbacks) + { + await action(context).ConfigureAwait(context.ContinueOnCapturedContext); + } } finally { @@ -148,12 +117,12 @@ public async Task CloseAsync(CancellationToken cancellationToken = default) } } - private sealed class RegistrationDisposable : IDisposable + private sealed class RegistrationDisposable(CircuitBreakerManualControl owner, Func onIsolate, Func onReset) : IDisposable { - private readonly Action _disposeAction; - - public RegistrationDisposable(Action disposeAction) => _disposeAction = disposeAction; + private readonly CircuitBreakerManualControl _owner = owner; + private readonly Func _onIsolate = onIsolate; + private readonly Func _onReset = onReset; - public void Dispose() => _disposeAction(); + public void Dispose() => _owner.Remove(_onIsolate, _onReset); } } diff --git a/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs b/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs index 10a677b9658..bc00495036a 100644 --- a/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs +++ b/src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs @@ -104,7 +104,9 @@ public async ValueTask DisposeAsync() if (LoadedTasks == _maxAttempts) { await WaitForTaskCompetitionAsync().ConfigureAwait(ContinueOnCapturedContext); - return TryRemoveExecutedTask(); + var task = TryRemoveExecutedTask(); + Debug.Assert(task != null, "There must be a completed task after awaiting for an executing task"); + return task; } if (hedgingDelay == TimeSpan.Zero || LoadedTasks == 0) @@ -117,7 +119,9 @@ public async ValueTask DisposeAsync() if (hedgingDelay < TimeSpan.Zero) { await WaitForTaskCompetitionAsync().ConfigureAwait(ContinueOnCapturedContext); - return TryRemoveExecutedTask(); + var task = TryRemoveExecutedTask(); + Debug.Assert(task != null, "There must be a completed task after awaiting for an executing task"); + return task; } #if NET8_0_OR_GREATER @@ -143,7 +147,9 @@ await whenAnyHedgedTask.WaitAsync(hedgingDelay, _timeProvider, PrimaryContext!.C delayTaskCancellation.Cancel(); #endif - return TryRemoveExecutedTask(); + var completed = TryRemoveExecutedTask(); + Debug.Assert(completed != null, "There must be a completed task after awaiting for an executing task"); + return completed; } private ExecutionInfo CreateExecutionInfoWhenNoExecution() From 631fc87423441450ab59c4037170396a4758bafd Mon Sep 17 00:00:00 2001 From: Pent Ploompuu Date: Thu, 21 Aug 2025 19:32:16 +0300 Subject: [PATCH 10/10] Address PR feedback --- .../CircuitBreaker/Controller/CircuitStateController.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs index 8a2133265a3..e55fc2fc0b2 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs @@ -320,7 +320,11 @@ private void SetLastHandledOutcome_NeedsLock(Outcome outcome) private BrokenCircuitException CreateBrokenCircuitException() { TimeSpan retryAfter = _blockedUntil - _timeProvider.GetUtcNow(); - var exception = new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter, _breakingException!); + var exception = _breakingException switch + { + Exception ex => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter, ex), + _ => new BrokenCircuitException(BrokenCircuitException.DefaultMessage, retryAfter) + }; _telemetry.SetTelemetrySource(exception); return exception; }