Deleting entities asynchronously in C#

Question

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;
}

Answer 1

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.