2

i have a problem with threads, i have this code (example):

private void button_Click(object sender, EventArgs e) {
    ShowMessage("Starting Downloads...");
    <more code>
    StartDownloads();
    RunFileDownload();
    <more code>
}

private void StartDownloads() {
    <more code>
    for (int i=0; i<10; i++) {
        ShowMessage("Downloading file: " + i);
        Download(i);
        <more code>
    }
    <more code>
}

The problem is, when i press the button and the downloading starts, the messages are not displayed... I tried to fix it with threads, like this:

private void button_Click(object sender, EventArgs e) {
    ShowMessage("Starting Downloads...");
    Thread t = new Thread(new ThreadStart(StartDownloads));
    t.Start();
    RunFileDownload();
}

But the RunFileDownload(); function starts before the files are downloaded. I try solve this with "Thread.Join();" but again not displayed messages (The main thread is paused).

I thought solve it with a multi-thread and Thread.Join(); but it isn't efficient and i will have problems with others functions in the main thread.

How can i solve this problem? Thanks.

Edit #2:

Considering this code:

private void Download() {
    ShowMessage("Starting Downloads...");
    Thread t = new Thread(new ThreadStart(StartDownloads));
    ShowMessage("Downloads Finished...");   | not run until
    RunFileDownload();                      | finished
    ShowMessage("Files Executed...");       | thread.
}

How can i expect the thread finish before the rest of the code is executed? I try with Thread.Join(); but it freezes the application.

Manux22
  • 323
  • 1
  • 5
  • 16
  • are you missing some `{` in your code..? also try to make the methods that you are using `async` – MethodMan Jan 08 '15 at 23:19
  • @MethodMan yes, sorry, i wrote this code for the post, is an "example" of my code. Ok i go to search info for `async`. I'm new in this topic. Thanks – Manux22 Jan 08 '15 at 23:27
  • 1
    You might consider [WebClient.DownloadFileAsync](http://msdn.microsoft.com/en-us/library/system.net.webclient.downloadfileasync(v=vs.110).aspx) or [DownloadFileTaskAsync](http://msdn.microsoft.com/en-us/library/system.net.webclient.downloadfiletaskasync(v=vs.110).aspx). The `WebClient` class is very useful for simple HTTP transactions. – Jim Mischel Jan 09 '15 at 16:48

1 Answers1

5

Assuming you have access to async/await, the simplest solution is this:

private async void button_Click(object sender, EventArgs e)
{
    ShowMessage("Starting Downloads...");
    await StartDownloads(); //Return control until this method completes!
    RunFileDownload();
}

Note that exceptions with await are, suffice it to say, less than kind. Please ensure that you are using proper try/catch blocks, especially from await onwards. Consider using this pattern: Fire-and-forget with async vs "old async delegate" and reading this article.

Note that StartDownloads needs to be async and return a Task for this to work.

Apart from that solution, you need the thread to invoke a callback or raise an event on completion so you can run RunFileDownload. Using a BackgroundWorker can simplify that process.

Community
  • 1
  • 1
BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
  • Never thought of using async on button events. That's pretty cool – SimpleVar Jan 08 '15 at 23:23
  • @YoryeNathan In all fairness, its a trick Reed Copsey taught me a few weeks ago. It is impressively useful. – BradleyDotNET Jan 08 '15 at 23:23
  • Is there any downside to doing this in your opinion, compared to calling an async method in the synchronous event? – SimpleVar Jan 08 '15 at 23:24
  • @YoryeNathan You mean not making the event async? You can't use `await` unless its marked that way, which you need for what you want. If you are suggesting just calling a *different* async method, that would be fine. No real difference AFAIK, other than you could continue the event handler while the async code ran – BradleyDotNET Jan 08 '15 at 23:25
  • Yeah, I meant the latter. I suppose it's just a matter of what you wanna do. Cool to know anyways, thanks ;3 – SimpleVar Jan 08 '15 at 23:27
  • 2
    @YoryeNathan Don't forget that `async void` should only be used for events, like this; when calling a method returning `void`, a `Task` should be returned, not `void`, otherwise your error capturing is gone. – dyson Jan 08 '15 at 23:30
  • 1
    @barrick `async void`is also very useful when you want a "fire and forget" method as well. Otherwise I agree with you – BradleyDotNET Jan 08 '15 at 23:31
  • @BradleyDotNET - yes, but "fire and forget" should only be used for UI events. See http://stackoverflow.com/questions/12803012/fire-and-forget-with-async-vs-old-async-delegate and especially linked http://www.jaylee.org/post/2012/07/08/c-sharp-async-tips-and-tricks-part-2-async-void.aspx for more info. – Alexei Levenkov Jan 08 '15 at 23:42
  • @Manux22 You have to actually invoke the method. It won't work against a delegate. `await downloader.StartDownloads();` – BradleyDotNET Jan 08 '15 at 23:47
  • @BradleyDotNET I don't know Reed's motivation (and I perfectly sure he or Skeet can write such code correctly), but for "async" code I'd listen to advice by Stephen Cleary over pretty much anyone else... If you do handle exceptions correctly and don't let them get lost it is reasonable to fire and forget - but code will *look* wrong/dangerous and I'd try avoid it. – Alexei Levenkov Jan 08 '15 at 23:56
  • @AlexeiLevenkov Fair enough. I'm actually discussing it with him right now and he has clarified his stance and it aligns with what you said. – BradleyDotNET Jan 08 '15 at 23:57
  • @BradleyDotNET yes yes, sorry, in the function "StartDownloads" i changed `void` for `async Task` but it show a warning "missing await operators", i don't need use `await` in this function..., and `Task` need a return, but i don't need use return in this function (I set `return "ok";` to solve the error) how can i solve this? Sorry for my bad english :/ – Manux22 Jan 08 '15 at 23:59
  • @Manux22 I'm not sure you need a return based on the docs, but I would just return a `new Task`. You shouldn't be missing any await operators if the event is `async void` which as we discussed is OK for UI events. – BradleyDotNET Jan 09 '15 at 00:01
  • 1
    @AlexeiLevenkov Just to update you after my conversation with Reed, he uses `async void` in exactly one place, as an extension method to `Task` that awaits the task and catches/logs any exceptions. Then he invokes that extension method on standard Task returning asyncs, turning them into fire and forget methods. – BradleyDotNET Jan 09 '15 at 00:19
  • @BradleyDotNET - consider if adding that summary as answer to fire-and-forget question I've linked. – Alexei Levenkov Jan 09 '15 at 00:53
  • @AlexeiLevenkov Done: http://stackoverflow.com/a/27852439/1783619 Thanks for the suggestion! – BradleyDotNET Jan 09 '15 at 01:20
  • @Manux22 You may need to put the long running part of `StartDownloads` into a `await Task.Run(() => ...)` call. As to your other point, what is confusing you? I was just providing an alternate solution. – BradleyDotNET Jan 09 '15 at 01:23
  • @BradleyDotNET only issue is with the language. Just wrapping `async StartDownloads();` with a `try` would still kill the app if an exception occurs in `RunFileDownload();`. – Aron Jan 09 '15 at 01:46
  • @BradleyDotNET I had already tried using `await Task.Run (() => ...)` but it gives me error "'await' can not be used as an identifier within an asynchronous method or lambda expression" but the main problem is that the messages from `StartDownloads();` are not displayed in real time – Manux22 Jan 09 '15 at 02:14
  • @Manux22 If those methods need to be real time, then `async` might not be the best choice here. You need to raise those events to the UI thread for updating *while you are still executing*. `async` isn't quite built for that. – BradleyDotNET Jan 09 '15 at 03:02
  • @Manux22 For the messages shown by `Download`, async is a reasonable way of solving this. If you have messages *in* StartDownloads, then you need to use callbacks and events to control the program flow so that it actually can be in a separate thread. You could also move the async/await pattern *into* `StartDownloads` and that should work as well. Does that make sense? – BradleyDotNET Jan 09 '15 at 16:47