Reputation: 16310
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?
Upvotes: 1
Views: 118
Reputation: 10929
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.
Upvotes: 1
Reputation: 81503
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);
Upvotes: 1