0

I have a long operation that involves calling some async functions.

These work fine, but I'd like to keep my UI alive so the user can move the window and click a cancel button. To that end, I'm using BackgroundWorker.

However, I'm having issues where my code is still running after BackgroundWorker thinks the operation has completed. As best I can tell, it's because I'm using await within DoWork().

If so, what are the options here? How can I call these methods, which require await, and still leave my UI active? Is there an option in WinForms?

Update

Here's some code:

private async void backgroundWorker1_DoWork(object sender, System.ComponentModel.DoWorkEventArgs e)
{
    string path = Program.GetDocumentsSubfolder("Uploads");
    string[] files = Directory.GetFiles(path, "*.upload");

    if (files.Length > 0)
    {
        YouTubeApi api = new();
        api.StatusChanged += Api_StatusChanged;

        foreach (string file in files)
        {
            await api.UploadVideo(file);
        }
    }
}
Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • See following : https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.backgroundworker?view=net-6.0 – jdweng Aug 18 '22 at 21:44
  • This shouldn't happen. The code in `DoWork()` runs in a background thread and the code in `RunWorkerComplete()` should run after the former completes. Can't really say what's wrong without seeing the code. That said, consider ditching BackgroundWorker altogether and just use `Task.Run()` with async/await and the modern way of canceling tasks. [This blog post](https://blog.stephencleary.com/2013/05/taskrun-vs-backgroundworker-intro.html) by Stephen Cleary should be helpful. – 41686d6564 stands w. Palestine Aug 18 '22 at 21:52
  • 1
    You have async methods that block the UI? Not really async methods or you're flooding the UI. Code required -- A BackgroundWorker that executes async methods is nonsensical. – Jimi Aug 18 '22 at 21:54
  • @jdweng: Thanks, but I don't see where that article says anything about await. – Jonathan Wood Aug 18 '22 at 21:55
  • @Jimi; I use `await` within `DoWork()`. Is that what you mean by block the UI? – Jonathan Wood Aug 18 '22 at 21:56
  • Why do you need AWAIT? The example is already asynchronous. There is a complete event that you use to indicate when finish. – jdweng Aug 18 '22 at 21:57
  • @41686d6564standsw.Palestine: I haven't fully figured it out but, under the covers, I know that `await` does return before the task is done. I was wondering if that's what caused BackgroundWorker to assume the task was done. I will try `Task.Run()`. – Jonathan Wood Aug 18 '22 at 21:58
  • *I'd like to keep my UI alive so the user can move the window and click a cancel button*: async methods do not block the UI (unless, as mentioned, you're flooding the UI with the data coming from these methods - I hope you're not Invoking anything - or you actually do heavy work with the results of an async method) -- Code required. – Jimi Aug 18 '22 at 21:59
  • @JonathanWood : Have you every used BackGroundWorker. What you said is wrong. Look at example. You never use await inside DoWork. A winform has a block inside the constructor and you never need to block. – jdweng Aug 18 '22 at 21:59
  • @JonathanWood _"I know that await does return before the task is done"_ **Nope!** You've got it backwards. The whole point of `await` is to _wait_ until the task is finished _asynchronously_. Without seeing the code, it's pretty difficult to figure out the problem. Please edit the question and provide a [repro]. – 41686d6564 stands w. Palestine Aug 18 '22 at 21:59
  • @jdweng: I need await because the method I'm calling is async. I suppose I can use `Task.Run()` to call it. But I have no control over the method I'm calling. – Jonathan Wood Aug 18 '22 at 21:59
  • @jdweng: I've used BackgroundWorker many, many times. What exactly did I say that is wrong? – Jonathan Wood Aug 18 '22 at 22:00
  • You do not need to use Task.Run either. You use backgroundWorker1.RunWorkerAsync(); – jdweng Aug 18 '22 at 22:00
  • @41686d6564standsw.Palestine: No, internally await creates a state machine and returns and then resumes when the task is complete. This is all invisible to C#. – Jonathan Wood Aug 18 '22 at 22:01
  • @jdweng: I *am* using `RunWorkerAsync()`. But within `DoWork()`, I need to call an async method. How do you propose I do that without `await` or `Task.Run()`? – Jonathan Wood Aug 18 '22 at 22:02
  • @JonathanWood : When you use backgroundWorker1.RunWorkerAsync(); it is already async and runs until you return from worker. Since you are running Async code runs as a separate task and allows the rest of the code to run in parallel. It also doesn't lock the form so you can use the controls while BackGroundWorker is running. I often use BackGroundWorker with Winform so the controls do not get locked. I also use backgroundWorker1_ProgressChanged to send data to form so I do not get cross threading issues. – jdweng Aug 18 '22 at 22:07
  • @jdweng: Yes, I'm well aware of that. But that didn't answer the question of how I can call async code within DoWork() without using await or Task.Run(). – Jonathan Wood Aug 18 '22 at 22:08
  • @JonathanWood : Why do you need to call an async inside another async? – jdweng Aug 18 '22 at 22:10
  • @jdweng: Because the method I must call (the whole point of this code) is an async method. If I call the async method without using BackgroundWorker, my UI is locked up. – Jonathan Wood Aug 18 '22 at 22:10
  • @JonathanWood : There is no reason to ever call an async inside another async. It does make a lot of sense. The RunWorkerAsync allows the worker to run in parallel with the form. – jdweng Aug 18 '22 at 22:12
  • 2
    Good to see your trolling game is still going strong, @jdweng, especially _"There is no reason to ever call an async inside another async"_ had me in stitches, please keep it up. There's of course a difference between the "Begin/Complete" functionality of .NET predating async/await, and you're pretending they're one and the same. They're not. You can only await from a method that has the `async` modifier, and that's not what RunBackgroundWorkerAsync() is. – CodeCaster Aug 18 '22 at 22:14
  • @jdweng: But the one question I have there, you can't answer. I can ask Google if they will change their API NuGet package to not be async, but my hunch is they aren't going to change that for me. – Jonathan Wood Aug 18 '22 at 22:14
  • @JonathanWood BackgroundWorker was useful before async/await (before .NET 4.5). If you're already using async/await, you don't need a BackgroundWorker because async/await (combined with `Task.Run()` for CPU-bound operations) allow you to do everything you would do with a BackgroundWorker as explained in the blog post linked above. That being said, there's nothing preventing you from using `await` inside the `DoWork` event; it should work just fine. Now, you're saying that it's not working fine with you, so we need a reproducible example. What you provided isn't complete and isn't reproducible. – 41686d6564 stands w. Palestine Aug 18 '22 at 22:14
  • @41686d6564standsw.Palestine: If I use async without BackgroundWorker, it blocks my UI. That's what I'm trying to address. From what I can tell, I can use await within DoWork, but it causes problems. If instead of await, I use `api.UploadVideo(file).Wait()`, then things seem to work normally. – Jonathan Wood Aug 18 '22 at 22:16
  • @JonathanWood : I've used RunWorkerAsync lot of times and it always solved the lock up issue. Make sure you only create one instance of the BackGroundWorker. The BackGroundWorker will give an error if you call it once it is running. It sounds like you may of create multiple instances or you sending data faster than the for can process the data. You may want to add timers inside the worker to slow down the processing. Adding an async isn't going to solve issues when worker is creating lots of data. – jdweng Aug 18 '22 at 22:18
  • _"If I use async without BackgroundWorker, it blocks my UI"_ That will only happen if the method you're calling uses synchronous code under the hood (i.e., isn't truly async). If that's the case, you can circumvent that by using `Task.Run()`, which is similar to using a BackgroundWorker. _"I can use await within DoWork, but it causes problems. If instead of await, I use api.UploadVideo(file).Wait(), then things seem to work normally"_ AFAIK, both `await` and `.Wait()` should work fine inside the `DoWork` event handler. Can you provide a complete example that would reproduce this problem? – 41686d6564 stands w. Palestine Aug 18 '22 at 22:20
  • @jdweng: I don't think you're following. I said it blocks my UI *WITHOUT* BackgroundWorker. My UI works great with BackgroundWorker. But the method I need to call is async. That's the issue. – Jonathan Wood Aug 18 '22 at 22:20
  • @41686d6564standsw.Palestine: `await` *does* work within the `DoWork()` handler. But I'm using report progress to propagate status messages. And those status messages are causing errors because BackgroundWorker thinks the task is already done when it isn't. I'm thinking of how I could possible create a small example that reproduced the issue but there's a lot going on. Since I have a workaround (the one I just mentioned), I may just go with that. – Jonathan Wood Aug 18 '22 at 22:25
  • Note that `await` and `.Wait()` should both work fine _only because you're in a background thread_ (i.e., inside `DoWork()`). With the main (i.e., UI) thread, `.Wait()` would cause a deadlock even if the method is truly asynchronous. – 41686d6564 stands w. Palestine Aug 18 '22 at 22:25
  • You real issue is when you repaint controls on a winform it takes a lot of time. It is best on a winform to minimize the number of times you send data to the control. For example it is best to use AddRange(array) rather than use Add(one item) and put Add inside a loop. the AddRange will repaint the control once while add will repaint everytime. In your case the Wait() is waiting for the entire video steam to be received before playing. So you are delaying the time it take to start viewing video. I would prefer to start seeing video immediately instead of waiting. – jdweng Aug 18 '22 at 22:27
  • @JonathanWood Yeah, there might be something wrong with the logic of reporting progress, which you didn't show. This shouldn't normally happen. Again, it all comes down to seeing a [repro]. Otherwise, we'd be just guessing. – 41686d6564 stands w. Palestine Aug 18 '22 at 22:28
  • You should post the code you have used without that BGW. Also a link to that API and what the handler of `api.StatusChanged += Api_StatusChanged;` is actually doing and how many times per second it's called. – Jimi Aug 18 '22 at 22:28
  • @JonathanWood Okay, I managed to [reproduce](https://pastebin.com/dtmHVLhU) the problem myself. You're right about `ReportProgress()` not working because of `await` (but you should've been more specific with the error). Your question is actually a duplicate of [this one](https://stackoverflow.com/q/28230363/8967612). – 41686d6564 stands w. Palestine Aug 18 '22 at 22:39
  • @41686d6564standsw.Palestine: Yes, I see. I suppose the error would've been helpful. Guess I thought it might be more clear to ask about if `await` is compatible with BackgroundWorker. I think I understand the issue. The only thing I don't understand is something you've said and that I've also read. And that is that `Task.Run()` can be used instead of BackgroundWorker. Given my need to report progress, I'm not sure that would work. Thanks for finding the similar question. – Jonathan Wood Aug 18 '22 at 22:44
  • @JonathanWood Yes, it would. You'll just have to use `IProgress` instead of `ReportProgress()` and the `ProgressChanged` event. And for cancelation, you'd use a `CancellationToken`. This is all explained in the blog post that I linked to in my first comment. Check [the sections on the left](https://i.stack.imgur.com/vf8YK.png) of the blog post. Trust me, once you familiarize yourself with this method, you'll never use a BackgroundWorker again. – 41686d6564 stands w. Palestine Aug 18 '22 at 22:47
  • @41686d6564standsw.Palestine: Yes, okay. I see where he goes into that. Cool. Thanks! – Jonathan Wood Aug 18 '22 at 22:52
  • A quick and dirty fix is to replace the `await api.UploadVideo(file);` with `api.UploadVideo(file).GetAwaiter().GetResult();`, and remove the `async` from the signature of the event handler. – Theodor Zoulias Aug 18 '22 at 22:53
  • 2
    OK I think you should scratch this all and start at the beginning. `BackgroundWorker` and `async` do not mix well, as the BW thinks your function is finished (due to the state machine). Instead go fully `async`: remove the BW entirely, and rewrite your `DoWork` function as a normal `async` function, and hand it off to the threadpool using `Task.Run`. Report progress using a `Progres` object, which will synchronize it back to the UI. – Charlieface Aug 19 '22 at 00:38
  • Somethink like this https://dotnetfiddle.net/nUIUtK – Charlieface Aug 19 '22 at 00:50
  • @TheodorZoulias That's not just dirty but terrible. Various issues arise from there. – Lex Li Aug 19 '22 at 00:54
  • @LexLi I wouldn't expect anything bad to happen. Just blocking a `ThreadPool` thread during the background work, which is what is expected from a `BackgroundWorker` anyway. What kind of issues do you have in mind? – Theodor Zoulias Aug 19 '22 at 01:21
  • @TheodorZoulias unless you didn't read something like https://stackoverflow.com/questions/39007006/is-getawaiter-getresult-safe-for-general-use – Lex Li Aug 19 '22 at 04:36
  • @LexLi *"`GetAwaiter().GetResult()` can deadlock when it's used in a one-thread-at-a-time context."* -- The `BackgroundWorker` runs the work on the `ThreadPool`, which is `SynchronizationContext`-free. I am not saying that `.GetAwaiter().GetResult()` is ideal, but let's not oversell its risks. When you know what you are doing, it's not risky. it's just slightly inefficient. – Theodor Zoulias Aug 19 '22 at 06:48
  • @TheodorZoulias when there are clearly better ways to write async/await code (like completely removing the background worker component), suggesting a pattern that can easily fall into traps isn't the best you can offer. – Lex Li Aug 19 '22 at 07:14
  • @LexLi it's a call of judgment. In the long run learning about async/await, `Task.Run`, `IProgress` etc is an investment that will certainly pay off. Rewriting large parts of existing legacy applications to make use of these newer technologies, is less clear whether it will pay off. In the short term, we know nothing about the OP's commitments and deadlines. – Theodor Zoulias Aug 19 '22 at 07:29
  • 1
    Further reading through your words, like "I have a long operation that involves calling some async functions", "but I'd like to keep my UI alive", and "I'm using BackgroundWorker", I think you are just walking down the wrong path. Proper use of async/await and TPL should allow your app to be quite responsive even with long operations running in the background (a sample can be found in https://halfblood.pro/how-to-replace-backgroundworker-with-async-await-and-tasks-80d7c8ed89dc). `BackgroundWorker` is the legacy before TPL and shouldn't be used any more. – Lex Li Aug 19 '22 at 07:42
  • The most urgent thing to fix the issues is to get correct understanding of the key concepts and how to troubleshoot performance issues the right way. That often requires a good course, a great book, or an experienced consultant to be by your side. Shattered pieces over the internet can rarely guide you in a systematic way, and confusion can easily form. – Lex Li Aug 19 '22 at 07:47
  • @JonathanWood : To better understand the ASYNC inside an ASYNC you need to look at a simple two port app where the code moves data between two UARTS. Normally each UART would have its own interrupt handler and there would be buffers where one UART would fill and the other would remove. You would never put one interrupt handler inside another in this case which would be equivalent to putting an ASYNC inside another ASYNC. – jdweng Aug 19 '22 at 15:24

0 Answers0