Problem
I have a method that takes a collection of objects, and in turn calls a method that handles a single object. My question is, do I need to handle the tasks coming from the DeleteSingleAsync
method since it’s not really an async method or is this safe since the concrete implementation is just returning a completed task.
My initial gut concern is with exceptions/cancelation tokens not being handled correctly, or a slim possibility of a race condition in the tableTransactionActions
object.
private readonly IList<TableTransactionAction> tableTransactionActions;
public Task DeleteSingleAsync(TEntity entity, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
tableTransactionActions.Add(new TableTransactionAction(TableTransactionActionType.Delete, entity));
return Task.CompletedTask;
}
public Task DeleteAsync(IEnumerable<TEntity> entities, CancellationToken cancellationToken = default)
{
foreach (var entity in entities)
{
DeleteSingleAsync(entity, cancellationToken);
}
return Task.CompletedTask;
}
Solution
Not waiting in DeleteAsync
should be safe but I wouldn’t pass it from a code review perspective as in the future if DeleteSingleAsync
changes to be truly async and someone doesn’t know they also need to change DeleteAsync
bugs could come out of it. This is coding to the implementation and not the interface, which is bad practice IMO. We still don’t need to await
it in DeleteAsync
just do
return Task.WhenAll(entities.Select(entity => DeleteSingleAsync(entity, cancellationToken))
Now in the future if we change DeleteSingleAsync
to be async DeleteAsync
is still good to go.
If tableTransactionActions is a standard C# List<>
then it is not thread safe. Without knowing more of how this is used outside of this small snippet I would suggest to use the ConcurrentQueue
, or possibly ConcurrentStack
, instead of List
. But to know which Thread Safe class to use would need more information then the current code shown. Thread safety isn’t about async it’s about being called in parallel from multiple threads at the same time.