'Cancelling and creating tasks from different requests
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...
}
}
}
Solution 1:[1]
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.
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
| Solution | Source |
|---|---|
| Solution 1 | Theodor Zoulias |
