return in "wait Task<string>.Run (..)" sometimes hangs

Question

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

Answer 1

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