-2

I am writing below code in WPF. However, it does not run asynchronously. I have commented the line where the code gets blocked. Please let me know where am I doing the mistake.

private async void txtSampleRequest_Click(object sender, RoutedEventArgs e)
{
    await DownloadData();
}
        
async  Task<string> DownloadData()
{
    string authorization = txtAuthentication.Text;
    string cookie = txtCookie.Text;

    try
    {
        var vvv = Enumerable.Range(0, 50);

        List<Task> TaskList = new List<Task>();
        foreach (int s in vvv)
        {
            Task LastTask = ZerodhaOperations.MyHttpRequest("", authorization, cookie, "5minute");//Even this method is async method, and I have added dummy Task.Delay(1) to run method asynchronously. Below is the full definition of this class.
            TaskList.Add(LastTask);
        }

        await Task.WhenAll(TaskList);  //<------------ Here it stops working asynchronously and blocks UI of application. 
        MessageBox.Show("DONE");
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }

    return "";
}
class ZerodhaOperations
{
    public static async Task<string> MyHttpRequest(string WEBSERVICE_URL,string authorization, string cookie,string timeFrame)
    {
        await Task.Delay(1);
        if (authorization == "" || cookie == "" || authorization == null || cookie == null)
        {
            throw new ArgumentException("Aditya Says, No Authorization and cookie has been set");
        }
        string jsonResponse = "";
        WEBSERVICE_URL = "https://kite.zerodha.com/oms/instruments/historical/519937/timeFrame?user_id=YD0744&oi=1&from=2020-04-24&to=2020-04-24";
        WEBSERVICE_URL = $"https://kite.zerodha.com/oms/instruments/historical/806401/{timeFrame}?user_id=YD0744&oi=1&from=2020-12-03&to=2020-12-03";

        try
        {
            var webRequest = System.Net.WebRequest.Create(WEBSERVICE_URL);

            //ServicePointManager.SecurityProtocol = SecurityProtocolType.Ssl3;
            ServicePointManager.Expect100Continue = true;
            ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls
                   | SecurityProtocolType.Tls11
                   | SecurityProtocolType.Tls12
                   | SecurityProtocolType.Ssl3;
            ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };

            if (webRequest != null)
            {
                webRequest.Method = "GET";
                webRequest.Timeout = 5000;

                if (authorization != "")
                {
                    webRequest.Headers.Add("authority", "kite.zerodha.com");
                    webRequest.Headers.Add("authorization", authorization);
                }

                webRequest.Headers.Add("cookie", cookie);
                webRequest.Headers.Add("method", "GET");

                using (System.IO.Stream s = webRequest.GetResponse().GetResponseStream())
                {
                    using (System.IO.StreamReader sr = new System.IO.StreamReader(s))
                    {
                        jsonResponse = sr.ReadToEnd();
                        if (jsonResponse == "")
                            throw new Exception("Empty Response");

                        //MessageBox.Show(jsonResponse);
                    }
                }
            }
        }
        catch (Exception ex)
        {
            if (ex.Message.Contains("Empty Response"))
                throw ex;
            else
                MessageBox.Show(ex.ToString());
        }

        return jsonResponse;
    }
}
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
Aditya Bokade
  • 1,708
  • 1
  • 28
  • 45
  • 1
    You don't use benefits of asynchronous code. `Task.Delay(1)` is to quick to notice, after that code become synchronous. Make `Task.Delay(5000)` to see that UI is not blocked for 5 seconds. But after that `Task.Delay` your code is synchronous and will block the UI. – Fabio Dec 04 '20 at 07:04
  • 4
    Instead use asynchronous methods to retrieve data from web service. – Fabio Dec 04 '20 at 07:04
  • 6
    Drum roll.... Your HttpRequest is synchronous! I haven't used a `WebRequest` in eons, however if they have `async` methods use them. However, you should really be using `IHttpClientFactory` and be done with it – TheGeneral Dec 04 '20 at 07:09
  • if using an actually asynchronous call isn't an option, you can get around it by using Task.Run(() => someSyncMethod()) to force the sync operation to run on a different thread – The Lemon Dec 04 '20 at 07:15
  • You don't run tasks.... use `Task.Run( () => ZerodhaOperations.MyHttpRequest....)` – Michał Turczyn Dec 04 '20 at 07:42
  • Another thing, do not use async void as it will "fir and forget" the Task, event if it throws exception. It awlays is good practice to have some dummy variable to store the Task, eg. `_ = AsyncMehthod()` and define global event handler for dispatcher's event `UnobservedTaskException ` – Michał Turczyn Dec 04 '20 at 07:45
  • 2
    @MichałTurczyn - `async void` for UI controls event handlers are totally ok to use, exceptions will be thrown correctly. – Fabio Dec 04 '20 at 07:48
  • Here is a very old answer: https://stackoverflow.com/q/202481/60761 – H H Dec 04 '20 at 07:51
  • 2
    But you should of course switch to HttpClient and use its async methods. – H H Dec 04 '20 at 07:51

1 Answers1

1

The way await works is that it captures a "context" when it asynchronously yields. The details are in the link, but in this case the asynchronous method is running on the WPF UI thread, so it captures a context that will resume running on that UI thread.

So the await Task.Delay(1) is in fact forcing MyHttpRequest to act asynchronously, but it's what happens next that is tripping you up. MyHttpRequest does the await, which captures the UI context and returns an incomplete task. This happens multiple times in DownloadData, which collects the incomplete tasks and then awaits the Task.WhenAll of them. So then DownloadData returns an incomplete task which is awaited by the event handler, which returns control back to the WPF message loop.

Now, what happens next is that those Task.Delay timers fire off almost immediately. So, MyHttpRequest resumes its async method, and since its await captured that UI context, it resumes running on the UI thread. So the await Task.Delay(1) did cause a yield to the WPF message loop, but then practically the very next thing the UI thread has to do is resume those async methods.

And the remainder of those async methods are synchronous, blocking the UI thread.

So, to solve this, you can make the methods truly asynchronous. Delete the await Task.Delay(1) completely and instead replace the synchronous APIs with asynchronous ones (GetResponseAsync, ReadToEndAsync). Or if you want to modernize the code further, replace WebRequest with HttpClient.

Your other option is to keep the code synchronous and just use Task.Run to run the synchronous code on background threads. Again, you would delete the await Task.Delay(1), and this time you would change the method signature to be synchronous. Then you can wrap the call to the (synchronous) MyHttpRequest in a Task.Run, e.g., Task LastTask = Task.Run(() => ZerodhaOperations.MyHttpRequest("", authorization, cookie, "5minute")); The Task.Run solution is less ideal than the asynchronous solution, but it is acceptable if you have a lot of other code that still needs MyHttpRequest to be synchronous for now.

Note that either way, the usage of Task.Delay(1) is wrong. It does not "run [the] method asynchronously". The proper way to run a method asynchronously is to have it call asynchronous APIs and not call blocking APIs.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810