Aspnetcore: Cancellation Token parameter in BackgroundService.StartAsync is not respected

[CameronAavik]

I have a suspicion that this is by design but wanted to check. I was planning to unit test an implementation of the background service I had written. To stop it looping forever in the case of a bug in my code I was looking to cancel the service after a certain amount of time. I thought that by passing in a cancellation token into the StartAsync and then cancelling it after 1 second, that this would work but it seems that this cancellation token is ignored.

The relevant code is found here, and I've pasted it below for convenience.

/// <summary>
/// Triggered when the application host is ready to start the service.
/// </summary>
/// <param name="cancellationToken">Indicates that the start process has been aborted.</param>
public virtual Task StartAsync(CancellationToken cancellationToken)
{
    // Store the task we're executing
    _executingTask = ExecuteAsync(_stoppingCts.Token);

    // If the task is completed then return it, this will bubble cancellation and failure to the caller
    if (_executingTask.IsCompleted)
    {
        return _executingTask;
    }

    // Otherwise it's running
    return Task.CompletedTask;
}

The cancellationToken parameter is not used and it instead passes in a different cancellation token that is private to BackgroundService. I think it might be useful to combine the parameter cancellation token with _stoppingCts. I understand that I can call Dispose or StopAsync to cancel _stoppingCts, but that doesn't let me call CancelAfter for the behaviour I was looking for and doesn't change that the cancellation token is ignored.

[Tratcher]

The assumption here is that the StartAsync CT only applies for the duration of StartAsync, where the background service continues beyond it. It's also assumed that ExecuteAsync doesn't block synchronously. Do you have a case where ExecuteAsync is blocking StartAsync and needs to be cancelled?

[CameronAavik]

The service we have written isn't intended to block synchronously, however we had a scenario where a bug in our code caused it to block synchronously leading the tests to run in an infinite loop so we were looking to add in the cancellation token with a cancel in case this were to happen again.

[CameronAavik]

One more thing to add onto here, in the case that ExecuteAsync does block synchronously, _executingTask is never assigned to the task because it never returns. That means that calling StopAsync doesn't cancel the _stoppingCts since _executingTask is null.

We have worked around this on our end by extending BackgroundService to wrap all calls to ExecuteAsync with a Task.Run so that it always yields even if the code is blocking such as so.

protected override sealed async Task ExecuteAsync(CancellationToken stoppingToken)
{
    var serviceTask = Task.Run(async () => await RunAsync(stoppingToken));
    await Task.WhenAny(serviceTask, Task.Delay(Timeout.Infinite, stoppingToken));
}

protected abstract Task RunAsync(CancellationToken stoppingToken);

If you're fine with leaving the service as only supporting background services that don't block synchronously then I'd be all fine with closing this issue. But doing the above could support background services that block synchronously as well. I don't have a deep understanding of how async works just yet so if there are any issues with wrapping everything in a Task.Run that you can think of I'd be interested to hear about them.

Lastly, the documentation doesn't seem to make any mention of this restriction on background services, only by reading the code were we able to figure this out, so might be worth adding to the documentation?

[davidfowl]

This came up during the omits review and we decided that code should just be asynchronous. Maybe that was a poor assumption but we should at the very least document that. Task.Run is fine but it isn’t appropriate for blocking work unless the long running flag is passed.

[destumme]

We have had to work around this same issue as well. With the additional note that when our equivalent of RunAsync from @CameronAavik's example above returns a completed task (from unit test mocking or no-op code paths) it is an especially interesting issue to understand, because we thought we were using BackgroundSevice as intended. In those situations, it appears as though a state machine is not created because the compiler has optimized it to not need the state machine and it will run synchronously, so RunAsync never yields and _executingTask is never assigned.

[davidfowl]

Yes, we're going to fix some of this in 3.0, see https://github.com/aspnet/Hosting/issues/1602

[valxv]

I also implemented a workaround, by linking StartAsync cancellationToken to _stoppingsCts by adding this as a first line of StartAsync() method:

_stoppingCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);

And now in unit tests we can use:

var ctSource = new CancellationTokenSource(100);
await sut.StartAsync(ctSource.Token);
serviceMock.Verify(m => m.ProcessAsync(), Times.AtLeastOnce);

And yes, I copied BackgroundService.cs implementation to our project.

[muratg]

Closing this. https://github.com/aspnet/Extensions/issues/813 is tracking work we may potentially do here.

[AndyPook]

The simplest workaround is to add await Task.Yield; as line one of ExecuteAsync.

If everything actually returns synchronously then the method never yields and StartAsync does not complete.

Another visible side-effect of this problem is that if you have a background service in an aspnet project then you never see the "Application started" logging on the console. Indicating that the bootstrap process is blocked.