0

I am implementing a run() function that can not be async. The goal is to send a get request to a server, and then download some files based on the result of the get request and return the number of files downloaded. I have written an async function to do that but I want to essentially "await" it before the rest of my main function continues. I am unable to achieve this behavior currently as the function just hangs. Not sure why its hanging :(

I think I just need some insights into Task and why this isn't working as expected. I am familiar with promises in JS so I thought this wouldn't be that difficult.

Thank you!

public int run(){
    FilesManager test = new FilesManager();
    string path = Path.Combine("C:\Users\username\Documents", "Temp");
    Task<int> T_count = test.Downloadfiles(path); //TODO: trying to "await" this before the messageBoxes
    Task.WaitAll(T_count);
    int count = T_count.Result;
    MessageBox.Show("File Downloaded");
    MessageBox.Show(count.ToString());
}

public async Task<int> Downloadfiles(string path)
{
    String[] response = await getClient.GetFromJsonAsync<string[]>("http://localhost:3000");
    int counter = 0;
    try
    {
        foreach (string url in response)
        {
            Uri uri = new Uri(url);
            var response2 = await getClient.GetAsync(uri);
            using (var fs = new FileStream(
                path + counter.ToString(),
                FileMode.Create))
            {
                await response2.Content.CopyToAsync(fs);
            }
            counter++;
        }
        return counter;
    }catch(Exception e)
    {
        while (e != null)
        {
            MessageBox.Show(e.Message);
            e = e.InnerException;
        }
        return 0;
    }         
}

EDIT: Still not able to get the task.WaitAll(T_count) to work. With some more debugging, the execution jumps from the response2 = await getClient.GetAsync... straight into the waitAll, never hitting the copyToAsync or counter++.

Charlieface
  • 52,284
  • 6
  • 19
  • 43
Ari Baranian
  • 55
  • 1
  • 3
  • 1
    What's preventing your run function from being async? – mason Aug 14 '22 at 16:33
  • `WebClient ` <-- ew. [Don't use `WebClient`, use `HttpClient`](https://stackoverflow.com/questions/20530152/deciding-between-httpclient-and-webclient) - and if you can't use `HttpClient` because it's `async`-only then use `HttpWebRequest` (it's not as nice as `HttpClient` but it's still miles better than `WebClient`). – Dai Aug 14 '22 at 16:36
  • It just has to be sync for the plugin I am building. The Api requires a synchronous run function – Ari Baranian Aug 14 '22 at 16:36
  • 1
    @AriBaranian You can always run async methods in the thread-pool and use .NET's synchronization primitives to _safely_ wait until it's completed - or schedule it in the background and return immediately, assuming you'll be able to check-in on it in future. – Dai Aug 14 '22 at 16:37
  • @Dai I will look into HTTPClient for the file download. I am already using it for the "getClient" in my code but saw that webclient had a really easy way to do file download. But is that what is causing my problem? is the run() function set up correctly otherwise? – Ari Baranian Aug 14 '22 at 16:41
  • 2
    Take a look at what mister Cleary has to say about this: [Don't Block on Async Code](https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html) – Theodor Zoulias Aug 14 '22 at 17:14
  • 1
    @TheodorZoulias I'm surprised he hasn't shown-up in this thread in-person yet... – Dai Aug 14 '22 at 19:05
  • 1
    Consider using the AsyncEx library. Cleary is very good for this kind of stuff. See also https://learn.microsoft.com/en-us/archive/msdn-magazine/2015/july/async-programming-brownfield-async-development#the-thread-pool-hack – Charlieface Aug 14 '22 at 19:15

1 Answers1

1

Sync-over-async is a fundamentally difficult problem, because you need to guarantee that continuations never try to run on the thread you are blocking on, otherwise you will get a deadlock as you have seen. Ideally you would never block on async code, but sometimes that is not possible.

Task.Run(...)..GetAwaiter().GetResult() is normally fine to use for this purpose, although there are still some circumstances when it can deadlock.

Do not call the UI from inside the async function, therefore you must move the catch with MessageBox.Show to the outer function.

You can also make this more efficient, by using HttpCompletionOption.ResponseHeadersRead, and you are missing a using on the response2 object.

public int run()
{
    FilesManager test = new FilesManager();
    string path = Path.Combine("C:\Users\username\Documents", "Temp");
    
    try
    {
        int count = Task.Run(() => test.Downloadfiles(path)).GetAwaiter().GetResult();
        MessageBox.Show("File Downloaded");
        MessageBox.Show(count.ToString());
        return count;
    }
    catch(Exception e)
    {
        while (e != null)
        {
            MessageBox.Show(e.Message);
            e = e.InnerException;
        }
        return 0;
    }
}

public async Task<int> Downloadfiles(string path)
{
    String[] response = await getClient.GetFromJsonAsync<string[]>("http://localhost:3000");
    int counter = 0;
    try
    {
        foreach (string url in response)
        {
            using (var response2 = await getClient.GetAsync(url, HttpCompletionOption.ResponseHeadersRead))
            using (var fs = new FileStream(
                path + counter.ToString(),
                FileMode.Create))
            {
                await response2.Content.CopyToAsync(fs);
            }
            counter++;
        }
        return counter;
    }         
}

Another option is to remove and afterwards restore the SynchronizationContext, as shown in this answer.

Charlieface
  • 52,284
  • 6
  • 19
  • 43
  • I thought using `Task.Run(...).GetAwaiter().GetResult()` was only safe if you use `StartNew` with some non-default `TaskCreationOptions` values... but I might be getting confused.. – Dai Aug 14 '22 at 20:29
  • @Dai Then again, I might be getting confused also. Like I said, it's *hard*. – Charlieface Aug 14 '22 at 21:21