views:

490

answers:

5

When my C# application closes it sometimes gets caught in the cleanup routine. Specifically, a background worker is not closing. This is basically how I am attempting to close it:

private void App_FormClosing(object sender, FormClosingEventArgs e) { backgroundWorker1.CancelAsync(); while (backgroundWorker1.IsBusy) ; // Gets stuck here. }

Is there a different way that I should be doing this? I am using Microsoft Visual C# 2008 Express Edition. Thanks.

ADDITIONAL INFORMATION:

The background worker does not appear to be exiting. This is what I have:

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
   while (!backgroundWorker1.CancellationPending)
   {
      // Do something.
   }
}

I've also modified the cleanup code:

private void App_FormClosing(object sender, FormClosingEventArgs e)
{
   while (backgroundWorker1.IsBusy)
   {
      backgroundWorker1.CancelAsync();
      System.Threading.Thread.Sleep(1000);
   }
}

Is there something else that I should be doing?

+4  A: 

In the background worker thread you need to check the BackgroundWorker.CancellationPending flag and exit if it is true.

The CancelAsync() just sets this flag.

Or to put it another way. CancelAsync() doesn't actually cancel anything. It won't abort the thread or cause it to exit. If the worker thread is in a loop and checks the CancellationPending flag periodically it can catch the cancel request and exit.

MSDN has an example here although it doesn't use a loop in the worker routine.

Kevin Gale
close, but not quite. your background task should inspect [and respect!] its `DoWorkEventArgs.Cancel` property. unless you are sharing instance variables across threads - ill advised - background task will not have access to actual instance of `BackgroundWorker`.
johnny g
Microsoft's own example accesses the background worker via the sender parameter. And in any case I didn't refer to his variable for the background worker but to the property of the class. To make it more clear I added a link to an example.
Kevin Gale
A: 

Try:

if (this.backgroundWorker1.IsBusy) this.backgroundWorker1.CancelAsync();
JeremySpouken
+3  A: 

Kevin Gale is correct in stating that your BackgroundWorker's DoWork handler needs to poll for CancellationPending and return if a cancellation is requested.

That being said, if this is happening when your application is shutting down, you can just ignore it safely, as well. BackgroundWorker uses a ThreadPool thread, which is, by definition, a background thread. Leaving this running will not prevent your application from terminating, and the thread will automatically be torn down when your application shuts down.

Reed Copsey
+1 Absolutely, unless the background worker thread needs to perform some type of cleanup before it shuts down.
Kevin Gale
@Kevin: True, but since the OP was just trying to cancel the thread, it sounds like that's not the case...
Reed Copsey
Thanks, Reed. This appears to be a case where I was making something simple much more difficult than it need be.
Jim Fell
+1  A: 

This code is guaranteed to deadlock when the BGW is still running. BGW cannot complete until its RunWorkerCompleted event finished running. RunWorkerCompleted cannot run until the UI thread goes idle and runs the message loop. But the UI thread isn't idle, it is stuck in the while loop.

If you want the BGW thread to complete cleanly, you have to keep your form alive. Check this thread to see how to do that.

Hans Passant
+2  A: 

Some pretty good suggestions, but I don't believe they address the underlying issue: canceling a background task.

Unfortunately, when using BackgroundWorker, termination of your task depends on the task itself. The only way your while loop will terminate, is if your background task checks its Cancel property and returns or breaks from its current process.

Example Base

For example, consider

private readonly BackgroundWorker worker = new BackgroundWorker ();

public void SomeFormEventForStartingBackgroundTask ()
{
    worker.DoWork += BackgroundTask_HotelCalifornia;
    worker.WorkerSupportsCancellation = true;
    worker.RunWorkerAsync ();
}

// semantically, you want to perform this task for lifetime of
// application, you may even expect that calling CancelAsync
// will out and out abort this method - that is incorrect.
// CancelAsync will only set DoWorkEventArgs.Cancel property
// to true
private void BackgroundTask_HotelCalifornia (object sender, DoWorkEventArgs e)
{
    for ( ; ;)
    {
        // because we never inspect e.Cancel, we can never leave!
    }
}

private void App_FormClosing(object sender, FormClosingEventArgs e)     
{
    // [politely] request termination
    worker.CancelAsync();

    // [politely] wait until background task terminates
    while (worker.IsBusy);
}

This is what is happening by default. Now, maybe your task isn't an infinite loop, perhaps it is just a long-running task. Either way, your main thread will block [actually it is spinning, but whatevs] until the task completes, or doesn't as the case may be.

If you have personally written and can modify the task, then you have a few options.

Example Improvement

For instance, this is a better implementation of the above example

private readonly BackgroundWorker worker = new BackgroundWorker ();

// this is used to signal our main Gui thread that background
// task has completed
private readonly AutoResetEvent isWorkerStopped = 
    new AutoResentEvent (false);

public void SomeFormEventForStartingBackgroundTask ()
{
    worker.DoWork += BackgroundTask_HotelCalifornia;
    worker.RunWorkerCompleted += BackgroundTask_Completed;
    worker.WorkerSupportsCancellation = true;
    worker.RunWorkerAsync ();
}

private void BackgroundTask_HotelCalifornia (object sender, DoWorkEventArgs e)
{
    // execute until canceled
    for ( ; !e.Cancel;)
    {
        // keep in mind, this task will *block* main
        // thread until cancel flag is checked again,
        // so if you are, say crunching SETI numbers 
        // here for instance, you could still be blocking
        // a long time. but long time is better than 
        // forever ;)
    }
}

private void BackgroundTask_Completed (
    object sender, 
    RunWorkerCompletedEventArgs e)
{
    // ok, our task has stopped, set signal to 'signaled' state
    // we are complete!
    isStopped.Set ();
}

private void App_FormClosing(object sender, FormClosingEventArgs e)     
{
    // [politely] request termination
    worker.CancelAsync();

    // [politely] wait until background task terminates
    isStopped.WaitOne ();
}

While this is better, it's not as good as it could be. If you can be [reasonably] assured your background task will end, this may be "good enough".

However, what we [typically] want, is something like this

private void App_FormClosing(object sender, FormClosingEventArgs e)     
{
    // [politely] request termination
    worker.CancelAsync();

    // [politely] wait until background task terminates
    TimeSpan gracePeriod = TimeSpan.FromMilliseconds(100);
    bool isStoppedGracefully = isStopped.WaitOne (gracePeriod);

    if (!isStoppedGracefully)
    {
        // KILL! KILL! KILL!
    }
}

Alas, we cannot. BackgroundWorker does not expose any means of forceful termination. This is because it is an abstraction built on top of some hidden thread management system, one which could potentially destabalize other parts of your application if it were forcefully terminated.

The only means [that I have seen at least] to implement the above is to manage your own threading.

Example Ideal

So, for instance

private Thread worker = null;

// this time, 'Thread' provides all synchronization
// constructs required for main thread to synchronize
// with background task. however, in the interest of
// giving background task a chance to terminate gracefully
// we supply it with this cancel signal
private readonly AutoResetEvent isCanceled = new AutoResentEvent (false);

public void SomeFormEventForStartingBackgroundTask ()
{
    worker = new Thread (BackgroundTask_HotelCalifornia);
    worker.IsBackground = true;
    worker.Name = "Some Background Task"; // always handy to name things!
    worker.Start ();
}

private void BackgroundTask_HotelCalifornia ()
{
    // inspect cancel signal, no wait period
    // 
    // NOTE: so cheating here a bit, this is an instance variable
    // but could as easily be supplied via parameterized thread
    // start delegate
    for ( ; !isCanceled.WaitOne (0);)
    {
    }
}

private void App_FormClosing(object sender, FormClosingEventArgs e)     
{
    // [politely] request termination
    isCanceled.Set ();

    // [politely] wait until background task terminates
    TimeSpan gracePeriod = TimeSpan.FromMilliseconds(100);
    bool isStoppedGracefully = worker.Join (gracePeriod);

    if (!isStoppedGracefully)
    {
        // wipe them out, all of them.
        worker.Abort ();
    }
}

And that there, is a decent introduction on thread management.

Which is best suited for you? Depends on your application. It is probably best not to rock the boat, and modify your current implementation to ensure that

  1. your background task inspects and respects the Cancel property
  2. your main thread waits for completion, as opposed to polling

It is very important to compare and evaluate the pros and cons of each approach.

If you must control and guarantee termination of someone else's tasks, then writing a thread management system that incorporates the above may be the way to go. However you would lose out on out-of-box features like thread pooling, progress reporting, cross-thread data marshalling [worker does that, no?], and a bunch of other stuff. Not to mention, "rolling your own" is often error prone.

Anyway, hope this helps :)

johnny g