From c5b7dac7f236f2991f844a1c4cfe1d012f78a7fd Mon Sep 17 00:00:00 2001 From: tomasevicius Date: Wed, 1 May 2024 14:39:24 +0300 Subject: [PATCH 1/2] fix: Task.Delay with infinite timespan is neither awaited nor cancelled thus stays forever and leaks CancellationTokenRegistration --- .../Limiters/AdaptativeConcurrencyLimiter.cs | 12 ++++- .../Tasks/TaskExtensions.cs | 48 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs diff --git a/src/Farfetch.LoadShedding/Limiters/AdaptativeConcurrencyLimiter.cs b/src/Farfetch.LoadShedding/Limiters/AdaptativeConcurrencyLimiter.cs index e234872..aa9e7b6 100644 --- a/src/Farfetch.LoadShedding/Limiters/AdaptativeConcurrencyLimiter.cs +++ b/src/Farfetch.LoadShedding/Limiters/AdaptativeConcurrencyLimiter.cs @@ -59,11 +59,13 @@ public Task ExecuteAsync(Func function, CancellationToken cancellationToke /// public async Task ExecuteAsync(Priority priority, Func function, CancellationToken cancellationToken = default) { + using (var delayTaskCancellationSource = new CancellationTokenSource()) + using (var linkedCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, delayTaskCancellationSource.Token)) using (var item = await _taskManager.AcquireAsync(priority, cancellationToken)) { try { - var delayTask = Task.Delay(Timeout.Infinite, cancellationToken); + var delayTask = Task.Delay(Timeout.Infinite, linkedCancellationSource.Token); var resultTask = await Task.WhenAny( function.Invoke(), @@ -73,6 +75,14 @@ public async Task ExecuteAsync(Priority priority, Func function, Cancellat { cancellationToken.ThrowIfCancellationRequested(); } + else + { + // Stop delayTask, otherwise it is kept indefinetly and leaks CancellationTokenRegistration (DelayPromiseWithCancellation) + delayTaskCancellationSource.Cancel(); + + // await ensures cancellation was received and accepted before we dispose CancellationTokenSource + await delayTask.IgnoreWhenCancelled(); + } if (resultTask.IsFaulted && resultTask.Exception is not null) { diff --git a/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs b/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs new file mode 100644 index 0000000..5d71f61 --- /dev/null +++ b/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs @@ -0,0 +1,48 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Farfetch.LoadShedding.Tasks +{ + internal static class TaskExtensions + { + private static readonly Action IgnoreTaskContinuation = t => { _ = t.Exception; }; + + /// + /// Observes and ignores a potential exception on a given Task. + /// If a Task fails and throws an exception which is never observed, it will be caught by the .NET finalizer thread. + /// This function awaits the given task and if the exception is thrown, it observes this exception and simply ignores it. + /// This will prevent the escalation of this exception to the .NET finalizer thread. + /// + /// The task to be ignored. + public static void IgnoreExceptions(this Task task) + { + if (task.IsCompleted) + { + _ = task.Exception; + } + else + { + task.ContinueWith( + IgnoreTaskContinuation, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + } + + public static async Task IgnoreWhenCancelled(this Task task) + { + if (!task.IsCanceled) + { + try + { + await task.ConfigureAwait(false); + } + catch (OperationCanceledException) + { + } + } + } + } +} From f426edc3ff07879cef0f831cd419d8078f801f50 Mon Sep 17 00:00:00 2001 From: tomasevicius Date: Wed, 1 May 2024 14:46:50 +0300 Subject: [PATCH 2/2] Removed unused method --- .../Tasks/TaskExtensions.cs | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs b/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs index 5d71f61..1468c67 100644 --- a/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs +++ b/src/Farfetch.LoadShedding/Tasks/TaskExtensions.cs @@ -1,36 +1,10 @@ using System; -using System.Threading; using System.Threading.Tasks; namespace Farfetch.LoadShedding.Tasks { internal static class TaskExtensions { - private static readonly Action IgnoreTaskContinuation = t => { _ = t.Exception; }; - - /// - /// Observes and ignores a potential exception on a given Task. - /// If a Task fails and throws an exception which is never observed, it will be caught by the .NET finalizer thread. - /// This function awaits the given task and if the exception is thrown, it observes this exception and simply ignores it. - /// This will prevent the escalation of this exception to the .NET finalizer thread. - /// - /// The task to be ignored. - public static void IgnoreExceptions(this Task task) - { - if (task.IsCompleted) - { - _ = task.Exception; - } - else - { - task.ContinueWith( - IgnoreTaskContinuation, - CancellationToken.None, - TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, - TaskScheduler.Default); - } - } - public static async Task IgnoreWhenCancelled(this Task task) { if (!task.IsCanceled)