5

Updated with answers: The true way of wait until a number of different tasks to be finished would need async await instead of background worker.

#

I know there are numerous discussion about backgroundworker but I've being searched around and cannot find the answer.

Here is my code example(basic logic, the actual code is much longer), I wonder if there is a way to get around this:

 BackgroundWorker MCIATS1Worker = new BackgroundWorker();
    private AutoResetEvent _MCIATS1WorkerResetEvent = new AutoResetEvent(false);

    public MainWindow()
    {
        InitializeComponent();

        MCIATS1Worker = new BackgroundWorker();
        MCIATS1Worker.DoWork += new DoWorkEventHandler(MCIATS1Worker_DoWork);
        MCIATS1Worker.WorkerReportsProgress = true;
        MCIATS1Worker.WorkerSupportsCancellation = true;
        MCIATS1Worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(MCIATS1_RunWorkerCompleted);

        for (int i = 1; i <= 10; i++)
        {
            //some code
            MCIATS1Worker.RunWorkerAsync();
            _MCIATS1WorkerResetEvent.WaitOne();
        }

    }

DoWork and runworkercompleted

void MCIATS1Worker_DoWork(object sender, DoWorkEventArgs e)
    {
        //do something here
    }

    void MCIATS1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        MessageBox.Show("hello world");
        _MCIATS1WorkerResetEvent.Set();
    }

For some reasons, the MCIATS1_RunWorkerCompleted won't be triggered until the loop finished. And apparently the WaitOne is holding the loop.
Here is my question,

why RunWorkerCompleted won't be trigger the RunWorkerCompleted when the worker is actually finished the work?

Thank you.

###UPDATED SOLUTION

This is the right way of doing it.

private async void WhateverFunction()
   {
    await Task.WhenAll(MCIATS1WorkerDoWorkAsync(param),...other tasks);
    }


private Task MCIATS1WorkerDoWorkAsync(bkgWorkParameter param)
    {
        return Task.Run(() =>
        {
            //Do whatever
        });
    }
Heisenberg
  • 764
  • 6
  • 20
  • this [link](http://stackoverflow.com/questions/1333058/how-to-wait-correctly-until-backgroundworker-completes) had a good explanation of waiting for a background worker, although it might take some what of an overhaul based on what you have here now. – vipersassassin Apr 28 '17 at 21:26
  • Read this link before I posted the question. It was the loop that caused the problem. That post didn't explain/involve loops. – Heisenberg Apr 28 '17 at 21:31
  • Are you referring to your 'for()' loop? – vipersassassin Apr 28 '17 at 21:33
  • Your for loop adds nothing to the problem, comment the for statement out and run the two inner lines of code once and you will see the same deadlock. – Scott Chamberlain Apr 28 '17 at 21:38
  • You are waiting for the thread(s) to complete inside the constructor - totally negating the concurrency the Bgw offers. – H H Apr 28 '17 at 22:02

2 Answers2

4

It happens because when you use a BackgroundWorker it's RunWorkerCompleted event is posted to the SynchronizationContext of the thread that called RunWorkerAsync.

Because you call RunWorkerAsync on the UI thread the event can't run until the UI thread starts processing new messages in the message loop. However you prevented the UI thread from returning to the message loop by your _MCIATS1WorkerResetEvent.WaitOne(); call.

So what it boils down to is _MCIATS1WorkerResetEvent.Set(); is waiting for MCIATS1_RunWorkerCompleted to fire to stop blocking and MCIATS1_RunWorkerCompleted is waiting for _MCIATS1WorkerResetEvent.Set(); to stop blocking the UI thread so it's message to be processed.

Both things are waiting for the other to complete before itself completes and you have a classic deadlock.

There is no need for a for loop for this problem to happen, this same problem would happen with or without out the loop, in fact the loop never gets to run it's 2nd itteration because it will have deadlocked on the first time through so it does not matter that there is a loop at all.

Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • Thanks Scott. i see....it is not the loop causing the problem. So how do I make sure the backgroundworker finished work before I start the next for loop? – Heisenberg Apr 28 '17 at 21:44
  • 1
    Easy, put the loop of the work inside the background worker itself. Instead of starting 10 threads to do 1 thing each make 1 thread that does 10 things in a loop inside of it. – Scott Chamberlain Apr 28 '17 at 21:44
  • the for loop has other things in it and it cannot be put inside the background worker. In fact, I will need to put multiple backgroundworkers inside the loop....and the second iteration has to wait until all backgroundworkers in the first iteration finish – Heisenberg Apr 28 '17 at 22:01
  • 1
    In that case I would drop BackgroundWorker entirely and replace `RunWorkerAsync()` and your `_MCIATS1WorkerResetEvent` with `await Task.Run(() => \* What used to be in RunBackgroundRorker */);` (See Fabio's answer for a fuller code example) – Scott Chamberlain Apr 28 '17 at 22:09
3

Depend on what kind of work your MCIATS1Worker_DoWork method do, you can consider to use async-await approach, which makes code a little bid more cleaner.

private async Task MCIATS1WorkerDoWorkAsync()
{        
    await Task.Delay(1000) // do something asynchronously for 1 second
}

private async void MainWindow_Load(object sender, EventArgs e)
{
     for (int i = 1; i <= 10; i++)
     {
        //some code
        await MCIATS1WorkerDoWorkAsync();
        MessageBox.Show("hello world");
     }       
}

Message box will be shown 10 times every 1 second. await keyword will continue loop only after MCIATS1WorkerDoWorkAsync method has successfully finished.

With async-await your form will remain responsive and if DoWork method do some IO operations, then you will not start another thread (as BackgroundWorker do) and whole execution will happens on one thread.

Fabio
  • 31,528
  • 4
  • 33
  • 72
  • Thank you, Fabio. I am very new to the asyn-await method. Let me do some research how this works. If you have some good resource, please let us know too. – Heisenberg Apr 28 '17 at 22:23