Cancelling and creating tasks from different requests

Question

In an ASP.net core application I spawn background tasks in one request, which can finish by themselves, or be cancelled by a second request. I have the following implementation, but I feel there should be a better way of achieving what I want? Does somebody have some experience with this problem?

public sealed class SomeService
{
    private record BackgroundTask(Task Task,
        CancellationTokenSource CancellationTokenSource);

    private readonly ConcurrentDictionary<Guid, BackgroundTask> _backgroundTasks
        = new();

    public void Start(Guid id)
    {
        var cancellationTokenSource = new CancellationTokenSource();

        var cancellationToken = cancellationTokenSource.Token;

        var task = Task.Run(async () =>
        {
            try
            {
                await Task.Delay(1000, cancellationToken);
            }
            finally
            {
                _backgroundTasks.TryRemove(id, out _);
            }
        }, cancellationToken);

        _backgroundTasks.TryAdd(id, new BackgroundTask(task, cancellationTokenSource));
    }

    public void Cancel(Guid id)
    {
        _backgroundTasks.TryGetValue(id, out var backgroundTask);

        if (backgroundTask == null)
        {
            return;
        }
        
        backgroundTask.CancellationTokenSource.Cancel();

        try
        {
            backgroundTask.Task.GetAwaiter().GetResult();
        }
        catch (OperationCanceledException e)
        {
            // todo: cancellation successful...
        }
    }
}

How important are these tasks for the health of your application? If they are even mildly important, you should be aware that there is no guarantee about their completion. Take a look at this: [Fire and Forget on ASP.NET](https://blog.stephencleary.com/2014/06/fire-and-forget-on-asp-net.html) — Theodor Zoulias, Mar 18 '22 at 14:36
Also, as soon as you try to scale out, this will break very quickly. — Stephen Cleary, Mar 19 '22 at 00:33
Why do `TryAdd` and `TryGetValue` and, in both cases, ignore the return `bool`? Your code would be more robust if you used the return values. They're there for a reason. — Enigmativity, Mar 19 '22 at 23:08
Thank you for the link, it was very informative. Scalability will not be an issue, but the tasks are important. — Ynv, Mar 22 '22 at 12:51

score 2 · Accepted Answer · answered Mar 18 '22 at 14:55

There is a race condition in the code below:

var task = Task.Run(async () =>
{
    try
    {
        await Task.Delay(1000, cancellationToken);
    }
    finally
    {
        _backgroundTasks.TryRemove(id, out _);
    }
}, cancellationToken);

_backgroundTasks.TryAdd(id, new BackgroundTask(task, cancellationTokenSource));

It is theoretically possible that the task will be added in the _backgroundTasks after its completion, and so it will never be removed from the dictionary.

One solution to this problem could be to create a cold Task, and Start it only after it has been added in the dictionary. You can see an example of this technique here. Working with cold tasks is a low level and tricky technique, so be cautious!

Another solution could be to not use a dictionary at all, and identify the BackgroundTasks by reference instead of by id. You don't need to expose the internal BackgroundTask type. You can return an object reference of it, as shown here for example. But I guess that you have some reason to store the BackgroundTasks in a collection, so that you can keep track of them.

Cancelling and creating tasks from different requests

1 Answers1