How to correctly await a task within a task?

Question

I'm using Flurl to make requests. Currently I have the following code and everything works fine:

public class Request()
{
    public async Task<Bitmap> GetImage(string url)
    {
        using (var client = new Url(url)
        {
            var content = await client.GetStringAsync();
            //return as image
        }
    }
}

And on the main UI thread I do:

public async void GetImages()
{
    for (int i = 0; i < someCounter; i++)
    {
        var img = await myRequest.GetImage(url);
        Thread.Sleep(100);
    }
}

The above works but the UI thread sleeps for a while and I wanted to do the above within a task. So I did:

public async void GetImages()
{
    await Task.Run(async () => 
    {
        for (int i = 0; i < someCounter; i++)
        {
            var img = await myRequest.GetImage(url);
            Thread.Sleep(100);
        }
    });
}

However, with the above, the code stops at await client.GetStringAsync();. There's no exception, the code either stop execution there or waits for something.

Is what I'm trying to do correct or is there a better way. If it's correct, why does the inner await never completes?

Also why is this in a for loop anyway? you are just getting the same image `x` amount of time — TheGeneral, Oct 28 '18 at 05:46
@TheGeneral I need to make a slight delay between requests. This is a simplified version of the problem. In the actual method, I'm getting different images every request. — Ivan-Mark Debono, Oct 28 '18 at 05:46

score 1 · Answer 1 · edited Oct 28 '18 at 06:07

Don't use Thread.Sleep(100);, it wont await, it wont give the thread back, it will just sit there doing nothing and holding up your UI

If anything, you should be using Task.Delay, it will play nice with a async, it wont block the UI thread and works on a CallBack.

Also, you should let your async await pattern propagate like a virus up to the event which started this, by returning async Task (and only then using async void)

public async Task GetImages()
{
    for (int i = 0; i < someCounter; i++)
    {
        var img = await myRequest.GetImage(url);
        // for your small wait.
        await Task.Delay(100);
    }
}

If you are worried about all the state machines, in some cases you can just return the Task and not await.

Also, in your base library, there is no point returning to the captured context, so its suitable to callConfigureAwait(false);, this will give you a slight efficiency

var content = await client.GetStringAsync()
                          .ConfigureAwait(false);

Barr J · Accepted Answer · 2018-10-28T07:25:54.313

If you want to use sleep, you should probably use Task.Delay. This post will provide you with better insights on when to use Thread.Sleep and when to use Task.Delay.

Second, in your code, What you should do, instead of await and sleep, is the following:

public async void GetImages()
{
   var tasks = new List<Task>();
   for (int i = 0; i < someCounter; i++)
   {
      tasks.Add(myRequest.GetImage(url));
   }
   await Task.WhenAll(tasks).ConfigureAwait(false);
}

Notice the WhenAll call at the end of the for loop, which will create a task that will complete when all of the supplied tasks have completed.

Then, ConfigureAwait(false) tells the await that you do not need to resume on the current context (in this case, "on the current context" means "on the UI thread")

Third, in your call to the url, change it to Task.Run:

public class Request()
{
    public async Task<Bitmap> GetImage(string url)
    {
        using (var client = new Url(url)
        {
            //return as image
            var content = await Task.Run(() => client.GetStringAsync());
        }
    }
}

you use Task.Run to call the method, not as part of the implementation of the method.

How to correctly await a task within a task?

2 Answers2