59

I had such method:

public async Task<MyResult> GetResult()
{
    MyResult result = new MyResult();

    foreach(var method in Methods)
    {
        string json = await Process(method);

        result.Prop1 = PopulateProp1(json);
        result.Prop2 = PopulateProp2(json);

    }

    return result;
}

Then I decided to use Parallel.ForEach:

public async Task<MyResult> GetResult()
{
    MyResult result = new MyResult();

    Parallel.ForEach(Methods, async method =>
    {
        string json = await Process(method);    

        result.Prop1 = PopulateProp1(json);
        result.Prop2 = PopulateProp2(json);
    });

    return result;
}

But now I've got an error:

An asynchronous module or handler completed while an asynchronous operation was still pending.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
Sergino
  • 10,128
  • 30
  • 98
  • 159

4 Answers4

88

async doesn't work well with ForEach. In particular, your async lambda is being converted to an async void method. There are a number of reasons to avoid async void (as I describe in an MSDN article); one of them is that you can't easily detect when the async lambda has completed. ASP.NET will see your code return without completing the async void method and (appropriately) throw an exception.

What you probably want to do is process the data concurrently, just not in parallel. Parallel code should almost never be used on ASP.NET. Here's what the code would look like with asynchronous concurrent processing:

public async Task<MyResult> GetResult()
{
  MyResult result = new MyResult();

  var tasks = Methods.Select(method => ProcessAsync(method)).ToArray();
  string[] json = await Task.WhenAll(tasks);

  result.Prop1 = PopulateProp1(json[0]);
  ...

  return result;
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 2
    Why shouldn't you use prallel in ASP.NET? – Dirk Boer Sep 29 '14 at 18:32
  • 20
    @DirkBoer: Parallel code will dramatically reduce ASP.NET scalability, and interfere with its thread pool heuristics. It is only useful if you have parallelizable CPU-bound work to do and know for certain that you will only have a small number of concurrent users. – Stephen Cleary Sep 29 '14 at 18:40
  • I got the exact same exception as the OP and though there appeared to be a minor improvement in the response for a single user when using Parallel, I can see how it (Parallel.ForEach) may hit us unexpectedly in a live application – Sudhanshu Mishra Dec 18 '14 at 10:27
  • @StephenCleary i've seen your comments around SO on using Parallel code on the server. What do you consider a small number of users exactly or do you have any further reading on this statement? I'm working on a WCF service but curious if this applies to me – Dude0001 Nov 28 '16 at 21:19
  • @Dude0001: It depends on a ton of factors - the only way to know for your situation is to run load tests. Simple rule of thumb: if you don't need to, then don't. "Fast enough" is fast enough. – Stephen Cleary Nov 28 '16 at 21:39
  • 1
    If I have a large number of items to process, will this code not try to start all of them at the same time, requiring hundreds of threads? I imaging that limiting the level of multi-threading to something around the number of CPU cores would make things faster than trying to do everything at the same time, causing massive task switching overhead. – ygoe Apr 29 '17 at 11:26
  • 2
    @ygoe: "will this code try to start all of them at the same time" Yes. "requiring hundreds of threads?" [No](http://blog.stephencleary.com/2013/11/there-is-no-thread.html). – Stephen Cleary Apr 29 '17 at 11:44
  • IMHO `Parallel.ForEach` has a lot more advantages compared to using raw tasks. How about wrapping the `Parallel.ForEach` in a task? This will allow the use of `async` while getting the full benefits of using `Parallel.ForEach`. Something like: `Task.Factory.StartNew( () => Parallel.ForEach(items, item => DoSomething(item)));` – GETah May 02 '18 at 20:49
  • @GETah: `Parallel` has advantages *for parallel processing*. It doesn't work as well for asynchronous work items. Using `Task.Run(() => Parallel...)` is a great pattern for pushing parallel work off a UI thread. You wouldn't want to use it on ASP.NET, though. – Stephen Cleary May 03 '18 at 02:05
  • @StephenCleary, OP did not say anything about ASP.net. What about combining async & parallelism in a worker process? – Sean Oct 17 '18 at 17:27
  • @Sean The OP posted the error `An asynchronous module or handler completed while an asynchronous operation was still pending.`, which only happens on ASP.NET. Combining async and parallelism in a worker process is fine. – Stephen Cleary Oct 17 '18 at 18:02
  • @StephenCleary: Can I ask a question? What do you think about downloading many URLs, where much of the download task involves waiting? I'd like to download several URLs at a time. I thought Parallel might be the answer but it didn't like that my action method was awaitable. Any chance you could point me in the right direction? – Jonathan Wood May 17 '20 at 20:10
  • @JonathanWood: You can't use `Parallel` for asynchronous code; you'd want to use asynchronous concurrency, e.g., `Task.WhenAll`. There are lots of answers on SO about that approach. – Stephen Cleary May 17 '20 at 23:37
  • @StephenCleary: Okay, thanks. I've been searching around. But I wasn't really sure which direction I should go. I'll look into `Task.WhenAll`. – Jonathan Wood May 18 '20 at 00:30
  • @StephenCleary I was running Parallel.ForEach but my code running inside would execute, but the program would continue and exit prematurely (me not knowing parallelism) but your answer of creating multiple tasks and using .WhenAll gave me exactly what I needed. Thank you very much, this was a lifesaver for the client project I'm working on! – jmath412 Mar 03 '22 at 18:15
  • 1
    @jmath412: Sounds like an `async` problem; `Parallel.ForEach` doesn't work with `async`. The new `Parallel.ForEachAsync` does, or `Task.WhenAll` should also work. – Stephen Cleary Mar 03 '22 at 18:17
19

.NET 6 finally added Parallel.ForEachAsync, a way to schedule asynchronous work that allows you to control the degree of parallelism:

var urlsToDownload = new [] 
{
    "https://dotnet.microsoft.com",
    "https://www.microsoft.com",
    "https://twitter.com/shahabfar"
};

var client = new HttpClient();

var options = new ParallelOptions { MaxDegreeOfParallelism = 2 };
await Parallel.ForEachAsync(urlsToDownload, options, async (url, token) =>
{
    var targetPath = Path.Combine(Path.GetTempPath(), "http_cache", url);

    var response = await client.GetAsync(url, token);
    // The request will be canceled in case of an error in another URL.

    if (response.IsSuccessStatusCode)
    {
        using var target = File.OpenWrite(targetPath);

        await response.Content.CopyToAsync(target);
    }
});
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Majid Shahabfar
  • 4,010
  • 2
  • 28
  • 36
13

Alternatively, with the AsyncEnumerator NuGet Package you can do this:

using System.Collections.Async;

public async Task<MyResult> GetResult()
{
    MyResult result = new MyResult();

    await Methods.ParallelForEachAsync(async method =>
    {
        string json = await Process(method);    

        result.Prop1 = PopulateProp1(json);
        result.Prop2 = PopulateProp2(json);
    }, maxDegreeOfParallelism: 10);

    return result;
}

where ParallelForEachAsync is an extension method.

Serge Semenov
  • 9,232
  • 3
  • 23
  • 24
5

Ahh, okay. I think I know what's going on now. async method => an "async void" which is "fire and forget" (not recommended for anything other than event handlers). This means the caller cannot know when it is completed... So, GetResult returns while the operation is still running. Although the technical details of my first answer are incorrect, the result is the same here: that GetResult is returning while the operations started by ForEach are still running. The only thing you could really do is not await on Process (so that the lambda is no longer async) and wait for Process to complete each iteration. But, that will use at least one thread pool thread to do that and thus stress the pool slightly--likely making use of ForEach pointless. I would simply not use Parallel.ForEach...

Peter Ritchie
  • 35,463
  • 9
  • 80
  • 98