0

I have been reasearching once again the async tasks. No mater how i set the Tasks, my application suffers from UI freeze all the time. I have the following code for downloading the string from a webpage:

internal string DownloadString(string URL)
{
      var result =  LoadCompanyContracts(URL);
      return result.Result;
}

internal async Task<string> LoadCompanyContracts(string URL)
{
Task<string> task2 = Task<string>.Factory.StartNew(() =>
{
     for (int i = 0; i <= 10000000; i++) Console.WriteLine(i);
     WebClient wc = new WebClient();
     string tmp = wc.DownloadString(new Uri(URL));
     return tmp;
});
return task2.Result;

}

When i execute this task and during the for loop the UI of my application is freezing. Even though i believe that this code should not freeze the UI i am not able to find a solution. I have tried many different options and really want to use tasks instead of threads or events with webclient async.

Info: I am using .net 4.5 for my project. The difference in my code is that these functions are inside a class library(don't know if it matters).

Is it possible to run this code without blocking the user interface with async await by calling the DownloadString function from my code? If not what are the alternatives(any good nuget packages)?

1 Answers1

5

The async keyword doesn't make something run asynchronously, it enables you to use await to await an already asynchronous operation. You need to use DownloadStringTaskAsync to truly download in an asynchronous manner:

internal async Task<string> LoadCompanyContracts(string URL)
{
    ....
    using(var wc = new WebClient())
    {
        string tmp = await wc.DownloadStringTaskAsync(new Uri(URL));
        return tmp;
    }
}

await by itself returns execution in the original execution context (ie the UI thread). This may or may not be desirable, which is why library code typically uses ConfigureAwait(false); and lets the final user of the library to decide how to await:

string tmp = await wc.DownloadStringTaskAsync(new Uri(URL))
                     .ConfigureAwait(false);

Finally, there's no point in awaiting if you are going to call .Result from the top-level function. There is no point in using await at all if you don't want to do use the method's result in your code. LoadCompanyContracts could be just:

internal Task<string> LoadCompanyContracts(string URL)
{
    ....
    using(var wc = new WebClient())
    {
        return wc.DownloadStringTaskAsync(new Uri(URL))
                 .ConfigureAwait(false);
    }
}

Oops

Typically, you don't need to use await at all if you just return the result of an asynchronous operation. The method could just return wc.DownloadStringTaskAsync(..); BUT that would cause the method to return and dispose the WebClient before download finishes. Avoiding the using block isn't a solution either, as it will let an expensive object like WebClient alive longer than necessary.

That's why HttpClient is preferable to WebClient: a single instance supports multiple concurrent calls, which means you can have just one instance eg as a field and reuse it, eg:

  HttpClient _myClient =new HttpClient();

  internal Task<string> LoadCompanyContractsAsync(string URL)
  {
      ....
      return _myClient.GetStringAsync(new Uri(URL))
                      .ConfigureAwait(false);
  }
}

You could get rid of your DownloadString since it doesn't do anything on top of LoadCompanyContracts. If it does use the result of LoadCompanyContracts, it should be rewritten as:

internal async Task<string> DownloadString(string URL)
{
  var result =  await LoadCompanyContracts(URL);
  //Do something with the result
  return result;
}

EDIT

The original answer used DownloadStringAsync which is a legacy method that raises an event when download completes. The correct method is DownloadStringTaskAsync

EDIT 2 Since we are talking about a UI, the code can be made asynchronous all the way to the top event handler by using the async void syntax for the handler, eg async void Button1_Click, eg:

async void LoadCustomers_Click(...)
{   
    var contracts=await LoaCompanyContracts(_companyUrls);
    txtContracts>Text=contracts;
}

In this case we want to return to the original thread, so we don't use ConfigureAwait(false);

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • You could also use await Task.Run(() => wc.DownloadString(...)) to force the action to be done on the background thread. – Amaury Levé Oct 21 '16 at 11:58
  • Hi Panagioti, both solutions return an error "Cannot await void". Maybe you meant string tmp = await wc.DownloadStringTaskAsync(new Uri(URL)); ? – Panagiotis Kanellopoulos Oct 21 '16 at 12:01
  • 1
    @Evangelink That would simply waste the thread. `DownloadStringAsync` *does* run on a background thread or rather, it uses IO completion ports so it doesn't waste a thread *at all* while waiting for the result. `Task.Run` on the other hand wastes a thread while waiting for a response. – Panagiotis Kanavos Oct 21 '16 at 12:02
  • 1
    @PanagiotisKanellopoulos oops, it should be [DownloadStringTaskAsync](https://msdn.microsoft.com/en-us/library/hh138334(v=vs.110).aspx). Fixed it – Panagiotis Kanavos Oct 21 '16 at 12:02
  • 1
    @PanagiotisKanellopoulos someone missed DotNetZone's meetups on TPL for the past 3 years .... – Panagiotis Kanavos Oct 21 '16 at 12:05
  • @PanagiotisKanavos hahaha that was a good one. Well i have tried all the solutions and still the UI freezes. I think i am mistaking something here. – Panagiotis Kanellopoulos Oct 21 '16 at 12:08
  • @PanagiotisKanavos You should use return await in your code, otherwise wc might get disposed before the execution actually finished, please refer this answer http://stackoverflow.com/a/19103343/6248956 – YuvShap Oct 21 '16 at 12:10
  • 1
    @PanagiotisKanellopoulos you should be using async all the way up to your UI layer - do not use `.Result` anywhere. Otherwise your code will deadlock. – Ant P Oct 21 '16 at 12:10
  • @SomeUser These are really bad news for my code. If i cannot use the Result then there is not clean way to get the downloaded string with async await – Panagiotis Kanellopoulos Oct 21 '16 at 12:14
  • 1
    @PanagiotisKanellopoulos why? `var result = await AsyncMethod()` gives you the result as `result` for the rest of the context of that method. `await Task` is the asynchronous equivalent of `Task.Result`. The point is that the caller has to be async as well (and its caller, and so on), which is a basic fundamental principle asynchrony - if you block on an async process it ceases to be async. – Ant P Oct 21 '16 at 12:15
  • @AntP this code will return the Task and i really need only the string at that specific point(i cannot rewrite the whole library moving the Task to the UI) – Panagiotis Kanellopoulos Oct 21 '16 at 12:17
  • @PanagiotisKanellopoulos You shouldn't use Result, and it can work just fine with async await, you just need to ensure you await the async method inside the using statement. – YuvShap Oct 21 '16 at 12:17
  • 1
    @PanagiotisKanellopoulos if you can't make your code async all the way up to the UI, then don't use async at all. It doesn't make sense for a method to be asynchronous when the caller isn't. It is conceptually impossible (except for fire-and-forget scenarios where the firing is still synchronous but triggers an async process). – Ant P Oct 21 '16 at 12:18
  • @PanagiotisKanellopoulos what are you trying to do? Why *can't* you use async all the way up? – Panagiotis Kanavos Oct 21 '16 at 12:21
  • Using async all the way up means that i will have to change many parts of my code which i don't want to do on an already tested code. Unfortunately i see no other way but accepting @PanagiotisKanavos answer and do it old fashioned with either Threading or Webclient downloadstringasync – Panagiotis Kanellopoulos Oct 21 '16 at 12:23
  • 1
    @PanagiotisKanellopoulos do you understand that the concept of asynchrony requires the consumer to also behave asynchronously? Otherwise there is no point to it at all. – Ant P Oct 21 '16 at 12:25
  • 1
    @PanagiotisKanellopoulos fix the code, don't cover up the bug. Because *that's* what this is. There is *NO* reason at all to use two threads to download one string. Your UI will *still* block. Your "tested" code already fails the asynchronous execution test. Just call `DownloadString` if you intend to block – Panagiotis Kanavos Oct 21 '16 at 12:27
  • Yeah, spinning up a background thread won't help anything when the UI thread is just going to sit and wait for it to finish. – Ant P Oct 21 '16 at 12:28
  • What about using Webclients DownloadAsync. I will give it a try and when i get some free time i will try to move up to async all the way. – Panagiotis Kanellopoulos Oct 21 '16 at 12:30
  • 1
    Look at it this way - when you use an async method, it says to the caller, "right, that's happening - I'll let you know when it's done." Right now your UI is saying "great, I'll wait for the result," and then just sitting there waiting. If you want your UI to continue working in the meantime, then it's the *UI* that needs to change. – Ant P Oct 21 '16 at 12:30
  • Writing another event handler isn't going to save you any time and the code will be a lot uglier – Panagiotis Kanavos Oct 21 '16 at 12:31
  • I'll have to talk to the [Swedish .NET user group](http://www.meetup.com/sweden-progressive-dot-net/) they cover too many advanced subjects and forgot the basics – Panagiotis Kanavos Oct 21 '16 at 12:33
  • I agree with you @PanagiotisKanavos. I will try it out then and let you know(even though it is a lot of work). – Panagiotis Kanellopoulos Oct 21 '16 at 12:33
  • 1
    Honestly, doing async all the way up shouldn't be that big a deal - how many layers deep is this code? – Ant P Oct 21 '16 at 12:34
  • @AntP if it's a badly written legacy application ... I think the Swedish UG should start covering basics. Sessions on TPL Dataflow and Akka.NET are great, but someone has to show the basics too. (And the Greek UG, too) – Panagiotis Kanavos Oct 21 '16 at 12:43
  • Doing that on the Click event of the UI worked. That way i used webclient downloadstring as is without task moving the async and await to the click event. Thank you all for your help – Panagiotis Kanellopoulos Oct 21 '16 at 13:18