runtime: BackgroundService appears to create tasks that will never complete

This line appears to create a task that will most likely never complete: https://github.com/aspnet/Extensions/blob/aa7fa91cfc8f6ff078b020a428bcad71ae7a32ab/src/Hosting/Abstractions/src/BackgroundService.cs#L65

Based on my own understanding of BackgroundService and the description of its use of cancellation in dotnet/extensions#1245, I don’t expect the CancellationToken to StartAsync or StopAsync to be marked as cancelled under normal operation. (This is why BackgroundService exists: it provides a more useful CancellationToken to ExecuteAsync than the one provided to StartAsync, which is helpful for tasks that run indefinitely.) That means the task created with this expression is likely to never complete:

Task.Delay(Timeout.Infinite, cancellationToken)

To resolve this, a new CancellationTokenSource could be created for the current scope:

using var stopCts = new CancellationTokenSource();
using var _ = cancellationToken.Register(stopCts.Cancel);

await Task.WhenAny(_executingTask, Task.Delay(Timeout.Infinite, stopCts.Token));

I can submit a PR if this fix makes sense.

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 28 (26 by maintainers)

Most upvoted comments

FYI: We aren’t accepting new PRs for 3.0 right now and there isn’t a branch for 3.1 (the next release after 3.0) yet. I would expect one to be up and running sometime next week though. Just want to set the expectations. We’re starting to lock down for 3.0’s release later in September.

I think a PR would be OK (though would want input from @Tratcher and @davidfowl). I think that using a separate “stop the timeout” token would be preferred rather than relying on the fact that chaining the tokens like you did above means that when the method ends the timeout will cancel (because the CTS is disposed).

using var cancelTimeoutCts = new CancellationTokenSource();
using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancelTimeoutCts.Token, cancellationToken);

await Task.WhenAny(_executingTask, Task.Delay(Timeout.Infinite, linkedSource.Token));
cancelTimeoutCts.Cancel(); // Unconditionally stop the Task.Delay, if it's still running.

analogrelay on Jul 26, 2019

@moonrockfamily I blogged about that recently - part of a 3-part blog series. I use all three techniques to create a “critical BackgroundService” base type that avoids startup delay (Task.Run), catches ExecuteAsync exceptions, and logs and then exits the host if a critical background service fails.

StephenCleary on Jun 14, 2020

It might be possible to mitigate this with a more descriptive name for the methods.

I use WaitAsync(CancellationToken), in an attempt to make it clear that it is the wait that is cancelled and not the task.

StephenCleary on Mar 29, 2020

I think the solutions in your extension method are sufficient for our code, you can create a PR with that. I agree that the stuff in Microsoft.VisualStudio.Threading is a little more complicated than we need here (it does a lot of work to avoid allocating extra async state machines).

analogrelay on Aug 1, 2019

Would it be worth proposing to add this to corefx?

Like this? https://github.com/dotnet/corefx/issues/33007 😃

analogrelay on Aug 1, 2019

it’s either leaking, or it’s not

Fair point.

That seems to work:

Sounds good. Probably the best option since it’s clear as to what’s happening.

analogrelay on Jul 29, 2019