5

I ran into this piece of code:

items.ForEach(async item =>
{
     doSomeStuff();   
     await mongoItems.FindOneAndUpdateAsync(mongoMumboJumbo);
     await AddBlah(SqlMumboJumbo);
});

Is there any point in making this a .forEach delegate, or could it be just a normal foreach loop? As long as the function that contains the loop is in is async, this would be async by default?

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
VSO
  • 11,546
  • 25
  • 99
  • 187

2 Answers2

7

The delegate received by ForEach is an Action<T>:

public void ForEach(Action<T> action)

This means, that any async delegate you use inside it will effectively turn into an async void method. These are "fire and forget" style of execution. This means your foreach wont finish asynchronously waiting before continuing to invoke the delegate on the next item in the list, which might be an undesired behavior.

Use regular foreach instead.

Side note - foreach VS ForEach by Eric Lippert, great blog post.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • This is exactly what I am looking for. The reason I am on that controller method right now is because of multi-threading / threads trying to use the same db context errors. I don't really fully understand - but with the delegate, it tries to go through each item asynchronously, rather than in order? edit: haha, not the first time I end up at Eric's blog when it comes to C#. Will come back to accept answer once I get it, ty. – VSO Aug 11 '15 at 18:02
  • 1
    Good answer! Here is another useful link for OP: [Potential pitfalls to avoid when passing around async lambdas](http://blogs.msdn.com/b/pfxteam/archive/2012/02/08/10265476.aspx) – sstan Aug 11 '15 at 18:07
  • Just switching the code from items.ForEach to foreach loop seems to have fixed all my async errors. Will update if it fails e2e later, but it seems to have fixed it. Now to understand it fully.. – VSO Aug 11 '15 at 18:20
2

You don't know when your function is finished, nor the result of the function. If you start each calculation in a separate Task, you can await Task.WhenAll and interpret the results, even catch exceptions:

private async Task ActionAsync(T item)
{
    doSomeStuff();   
    await mongoItems.FindOneAndUpdateAsync(mongoMumboJumbo);
    await AddBlah(SqlMumboJumbo);
}

private async Task MyFunction(IEnumerable<T> items)
{
    try
    {
        foreach (var item in items)
        {
            tasks.Add( ActionAsync(item) )
        }
        // while all actions are running do something useful
        // when needed await for all tasks to finish:
        await Task.WhenAll(tasks);
        // interpret the result of each action using property Task.Result
    }
    catch (AggregateException exc)
    {
        ProcessAggregateException(exc);
    }
}

The aggregateException is thrown when any of your task throws an exception. If contains all exceptions thrown by all your tasks.

Harald Coppoolse
  • 28,834
  • 7
  • 67
  • 116