CODEWITHSUNDEEP
c#linqasync-await
Carlo V. Dango
Carlo V. Dango

Reputation: 13832

thread unsafe async in List<>.ForEach()

Why is it threadunsafe to do an await inside a .ForEach() ?

In a .Net Core 3.1 project I'm Selecting from some WebApi, the list of users matching a criteria, then I delete them, and finally I ask for the list of users again. Naturally, this time I expect the list to be empty.

The erroneous code

    var existing = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();

    existing.ForEach(async x => await client.DeleteByIdAsync(x.Id));

    var ensure = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();
    ensure.Count.Should().Be(0);  <-- ERROR WAS 1!

I've found that when I insert a Thread.Sleep(50) before var ensure the code works. This clearly indicates to me that there are threading issues going on I don't understand.

the working code (using a delay)

            var existing = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();

            existing.ForEach(async x => await client.DeleteByIdAsync(x.Id));
            Thread.Sleep(50);

            var ensure = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();
            ensure.Count.Should().Be(0);

Alternative Working code (using foreach)

            var existing = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();

            foreach (var x in existing)
            {
                await client.DeleteByIdAsync(x.Id);
            }

            var ensure = (await client.GetByMatchAsync(new SearchParameters() {..})).ToList();
            ensure.Count.Should().Be(0);

decompiling the .ForEach() I can't see where the threading problem occur

    public void ForEach(Action<T> action)
    {
        if (action == null)
        {
            ThrowHelper.ThrowArgumentNullException(ExceptionArgument.action);
        }

        int version = _version;

        for (int i = 0; i < _size; i++)
        {
            if (version != _version)
            {
                break;
            }
            action!(_items[i]);
        }

        if (version != _version)
            ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion();
    }

Upvotes: 2

Views: 232

Answers (1)

CRice
CRice

Reputation: 12567

List's ForEach just takes an Action<T> so the action you provide is not awaited per iteration, and execution can continue before the call is completed.

Otherwise it would likely have an overload with the signature

Task ForEach(Func<T, Task> func)

What I thought was strange is that the following line compiles, but you can't assign a Func<T, Task> to an Action<T>.

Action<string> a = async (value) => await Task.CompletedTask;

The fact you could invoke the ForEach this way was misleading.

existing.ForEach(async x => await client.DeleteByIdAsync(x.Id));

Upvotes: 5

Related Questions