0

This is a repro of my code, where the return in wait Task<string>.Run (..) sometimes hangs. If it fails, it's mostly on the first call.

How can I improve it?

using System.Threading.Tasks;
using System.Diagnostics;

private void Button_Click(object sender, RoutedEventArgs e)
{
    // This can be a very huge list
    string[] servers = new string[] { "10.17.100.1", "10.17.100.10", "10.17.100.20" };

    // the max parallel tasks must be limited
    Parallel.ForEach(servers,
        new ParallelOptions { MaxDegreeOfParallelism = 10 },  
        (forServer) =>
    {
        this.Method1Async(forServer).Wait();
    });

    Debug.WriteLine("Finished");
}

private async Task Method1Async(string server)
{
    await this.Method2Async(server);
}

private async Task Method2Async(string server)
{
    Debug.WriteLine("> Method2Async");

    string result = await Task<string>.Run(() =>
    {

        Debug.WriteLine("  Method2Async before return");

        return GetDataFromServer(server);
    });

    Debug.WriteLine("< Method2Async");
}

private string GetDataFromServer(string server)
{
    // any long time running stuff

    Thread.Sleep(10000);

    return "the server data";
}

Wanted output:

> Method2Async
  Method2Async before return
< Method2Async
Finished

Output when return hangs:

> Method2Async
  Method2Async before return
marsh-wiggle
  • 2,508
  • 3
  • 35
  • 52
  • 5
    Don't use `.Wait()` on tasks. – Lasse V. Karlsen May 31 '20 at 12:12
  • What do you mean "await the finish of the Parallel.ForEach"? It's a synchronous call, it doesn't return until all the parallel tasks has completed. – Lasse V. Karlsen May 31 '20 at 12:19
  • IIRC, await tells the compiler and runtime "reshuffle it so, that the rest of the function becomes a callback". Wait() is just a function call. Await is on par or even slightly above the brackets. It is easy to get them confused. – Christopher May 31 '20 at 12:20
  • please update your question, there is no `anydata` – Bizhan May 31 '20 at 12:46
  • You may find this interesting: [How to limit the amount of concurrent async I/O operations?](https://stackoverflow.com/questions/10806951/how-to-limit-the-amount-of-concurrent-async-i-o-operations). Also there is a nested `Parallel.ForEach` in the question, that looks like a transcript error. – Theodor Zoulias May 31 '20 at 15:55
  • @LasseV.Karlsen Now I agree and I am about to redesign my code. But what is Wait() for when it sometimes fails even in this very simple example? Or is there another pitfall in my code? – marsh-wiggle Jun 01 '20 at 07:39
  • 1
    @LasseV.Karlsen Why not use `.Wait()` on tasks? And do you also mean not to use `.Wait(timeSpan)`? (I'm asking because I have a `Task.Run(stuff).Wait(timeSpan)` which seems to hang and I'm not sure why.) – ispiro Apr 28 '22 at 18:20
  • @ispiro look at this: https://stackoverflow.com/a/62126997/1574221 I use it to work around that. – marsh-wiggle Apr 28 '22 at 18:41
  • 1
    .Wait and .Result and the similar synchronous ways of waiting for the task to complete can hang your code. The reasons are complex and not really suited for a comment length reply here, but you should find ample documentation and information about it. The end result is that .Wait and .Result should be reserved for library creators that create code that manages tasks (not just uses tasks), like extension methods or similar thing. Basically, if you know 100% how these things behave you can use them, otherwise you shouldn't. – Lasse V. Karlsen May 02 '22 at 06:16
  • @LasseV.Karlsen Thank you. It seems strange that Wait() was implemented the way it was, but I guess that's we have to work with... (By the way, If you don't add `@ispiro` to your comment, I don't get notified of your response.) – ispiro May 08 '22 at 18:34
  • 1
    @ispiro The core Task API was added before async/await was added, and so it had some flaws and rough edges. So yes, unfortunately it is what it is. – Lasse V. Karlsen May 09 '22 at 07:13
  • @LasseV.Karlsen I don't think they can get away with that excuse because `task.Wait();` is not (directly) related to aync/await. A simple Task that is `Wait()`ed can hang the application without any await. – ispiro May 09 '22 at 18:56

2 Answers2

2

Another option is to use ParallelForEachAsync of the apparently very popular AsyncEnumerator NuGet Package.

enter image description here

Awaitable foreach constructs can be realized by iteration of any IEnumerable or IAsyncEnumerable.

using Dasync.Collections;

string[] servers = new string[] { "10.17.100.1", "10.17.100.10", "10.17.100.20" };

await servers.ParallelForEachAsync<string>(async forServer =>
{
    await this.Method1Async(forServer);

}, maxDegreeOfParallelism: 10);

For collecting of return values a thread safe "bag" can be used.

using Dasync.Collections;

string[] servers = new string[] { "10.17.100.1", "10.17.100.10", "10.17.100.20" };

ConcurrentBag<string> bag = new ConcurrentBag<string>();

await severs.ParallelForEachAsync<string>(async forServer =>
{
    string response = await this.Method1Async(forServer);

    bag.Add(response);

}, maxDegreeOfParallelism: 10); 

foreach(string forBagItem in bag)
{
    // evaluate the results
}
marsh-wiggle
  • 2,508
  • 3
  • 35
  • 52
0

Note: thanks to Theodor Zoulias mentioning this:

According to this question Parallel.ForEach does not wait for the tasks to finish, therefore awaiting inside the action does not do anything and IsCompleted will be set to true as soon as all tasks are started.


Change the signature of the ForEach's Action to async to enable awaiting.

using System.Threading.Tasks;
using System.Diagnostics;

private void Button_Click(object sender, RoutedEventArgs e)
{
    string[] dummyArray = new string[] { "anyvalue" };

    Parallel.ForEach(dummyArray, async (forDummy) =>
    {
        await this.Method1Async();
    });

    Debug.WriteLine("Finished");
}

private async Task Method1Async()
{
    await this.Method2Async();
}

private async Task Method2Async()
{
    Debug.WriteLine("> Method2Async");

    string result = await Task<string>.Run(() =>
    {
        Debug.WriteLine("  Method2Async before return");
        return "anydata"; // this return sometimes does not "come back" ...
    });

    // ... so this code is never reached
    Debug.WriteLine("< Method2Async" + result);
}

Generally, when writing async code you must avoid synchronous calls (Wait, Result, etc.) or otherwise there is no point in writing asynchronous code. Just remove all Tasks, async and await and your code will run faster.

An exception to this rule is when you deliberately want to block thread, such as in legacy code.

Edit:

If you want to wait for all tasks to finish before going to the next statement, you can use WhenAll:

private async void Button_Click(object sender, RoutedEventArgs e)
{
    string[] dummyArray = new string[] { "anyvalue" };

    Task[] tasks = dummyArray.Select(async x => await Method1Async()).ToArray();
    await Task.WhenAll(tasks);
}

or

private async void Button_Click(object sender, RoutedEventArgs e)
{
    string[] dummyArray = new string[] { "anyvalue" };

    Task[] tasks = dummyArray.Select(x => Method1Async()).ToArray();
    await Task.WhenAll(tasks);
}

Edit:

If you want to limit the number of parallel tasks then you can do this:

    public async Task Button_Click()
    {
        string[] servers = new string[] { "1", "2", "3", "4", "5" };

        var maxParallel = 3;
        var throttler = new SemaphoreSlim(initialCount: maxParallel);
        var tasks = servers.Select(async server =>
        {
            try
            {
                await throttler.WaitAsync();
                await Method1Async(server);
            }
            finally
            {
                throttler.Release();
            }
        });
        await Task.WhenAll(tasks);

        Console.WriteLine("Finished");
    }
Bizhan
  • 16,157
  • 9
  • 63
  • 101
  • @marsh-wiggle You may instead, create a loop and check for `ParallelLoopResult.IsCompleted` but that's an overkill for this example. – Bizhan May 31 '20 at 12:36
  • @marsh-wiggle you said in `Wanted Output` that you want the second one gets called after the first one is finished. is that correct? – Bizhan May 31 '20 at 12:45
  • @marsh-wiggle I added another update. I hope it helps – Bizhan May 31 '20 at 12:57
  • 1
    The `Parallel.ForEach` [is not async-friendly](https://stackoverflow.com/questions/15136542/parallel-foreach-with-asynchronous-lambda). The lambda passed is [async void](https://learn.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming#avoid-async-void). – Theodor Zoulias May 31 '20 at 15:52
  • @marsh-wiggle please take a look at the updated answer again, and the comment above. It seems parallel foreach has a limitation. – Bizhan May 31 '20 at 20:00