How to stop BackgroundWorker on Form's Closing event?

Question

I have a form that spawns a BackgroundWorker, that should update form's own textbox (on main thread), hence Invoke((Action) (...)); call.
If in HandleClosingEvent I just do bgWorker.CancelAsync() then I get ObjectDisposedException on Invoke(...) call, understandably. But if I sit in HandleClosingEvent and wait for bgWorker to be done, than .Invoke(...) never returns, also understandably.

Any ideas how do I close this app without getting the exception, or the deadlock?

Following are 3 relevant methods of the simple Form1 class:

    public Form1() {
        InitializeComponent();
        Closing += HandleClosingEvent;
        this.bgWorker.RunWorkerAsync();
    }

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e) {
        while (!this.bgWorker.CancellationPending) {
            Invoke((Action) (() => { this.textBox1.Text = Environment.TickCount.ToString(); }));
        }
    }

    private void HandleClosingEvent(object sender, CancelEventArgs e) {
        this.bgWorker.CancelAsync();
        /////// while (this.bgWorker.CancellationPending) {} // deadlock
    }

Have you tried to use BegingInvoke instead of Invoke so you don't have to wait until the invokemessage returns? — Mez, Nov 13 '09 at 22:49
Yes. No dead lock, but I don't know when BeginInvoke has been processed (on the main thread), so I'm back to ObjectDisposed exception. — THX-1138, Nov 14 '09 at 17:15

Hans Passant · Accepted Answer · 2016-07-10T08:12:11.367

103

The only deadlock-safe and exception-safe way to do this that I know is to actually cancel the FormClosing event. Set e.Cancel = true if the BGW is still running and set a flag to indicate that the user requested a close. Then check that flag in the BGW's RunWorkerCompleted event handler and call Close() if it is set.

private bool closePending;

protected override void OnFormClosing(FormClosingEventArgs e) {
    if (backgroundWorker1.IsBusy) {
        closePending = true;
        backgroundWorker1.CancelAsync();
        e.Cancel = true;
        this.Enabled = false;   // or this.Hide()
        return;
    }
    base.OnFormClosing(e);
}

void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e) {
    if (closePending) this.Close();
    closePending = false;
    // etc...
}

edited Jul 10 '16 at 08:12

answered Nov 13 '09 at 22:41

Hans Passant

922,412
146
1,693
2,536

This works. I used workersThread.CancellationPending + workersThread.IsBusy flags, instead of mCompleted. – THX-1138 Nov 14 '09 at 17:23
8

That's kinda dangerous, IsBusy is a property of an asynchronous thread. It could race. It actually doesn't, but that was luck. Also, CancellationPending is reset before RunWorkerCompleted fires. – Hans Passant Jan 26 '10 at 20:33
3

Minor piece of information: you need to tell your BackGroundWorker instance that it can be cancelled. – Carlos Nov 16 '11 at 13:40
2

Speaking of races... Wouldn't this fail to close, if the worker happens to complete normally right after `if (!mCompleted)`? – Iain Sep 17 '12 at 04:37
7

@lain : No, OnFormClosing and backgroundWorker1_RunWorkerCompleted both run on the UI thread. One cannot be interrupted by the other. – Sacha K May 23 '13 at 13:36
Why is the "base.OnFormClosing(e);" there? – Shane Di Dona Aug 20 '15 at 03:43
@ShaneDiDona if the worker is completed or not even started it will close the form following the normal flow of events. – Ariel Moraes Feb 29 '16 at 17:22
Actually if I leave the base call there I get a StackOverflowException. – Ariel Moraes Feb 29 '16 at 20:54
I took me a while to realize that here the OnFormClosing is a method override, not an event handler. – denis_n Jul 16 '16 at 13:56
When I try this, the backgroundWorker1_RunWorkerCompleted doesn't finish until the form completes it's closing. I can't seem to get my application process to close out even when the form closes. Any advice? – ptn77 Mar 09 '17 at 23:45
Books I've been reading say flags are a product of code smell. But it's also what I've come up with and I don't see any other sensible way to handle it – mattshu Aug 09 '18 at 23:40

score 2 · Answer 2 · edited Mar 26 '15 at 06:16

Here was my solution (Sorry it's in VB.Net).

When I run the FormClosing event I run BackgroundWorker1.CancelAsync() to set the CancellationPending value to True. Unfortunately, the program never really gets a chance to check the value CancellationPending value to set e.Cancel to true (which as far as I can tell, can only be done in BackgroundWorker1_DoWork). I didn't remove that line, although it doesn't really seem to make a difference.

I added a line that would set my global variable, bClosingForm, to True. Then I added a line of code in my BackgroundWorker_WorkCompleted to check both e.Cancelled as well as the global variable, bClosingForm, before performing any ending steps.

Using this template, you should be able to close your form out at any time even if the backgroundworker is in the middle of something (which might not be good, but it's bound to happen so it might as well be dealt with). I'm not sure if it's necessary, but you could dispose the Background worker entirely in the Form_Closed event after this all takes place.

Private bClosingForm As Boolean = False

Private Sub SomeFormName_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
    bClosingForm = True
    BackgroundWorker1.CancelAsync() 
End Sub

Private Sub backgroundWorker1_DoWork(ByVal sender As Object, ByVal e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
    'Run background tasks:
    If BackgroundWorker1.CancellationPending Then
        e.Cancel = True
    Else
        'Background work here
    End If
End Sub

Private Sub BackgroundWorker1_RunWorkerCompleted(ByVal sender As System.Object, ByVal e As System.ComponentModel.RunWorkerCompletedEventArgs) Handles BackgroundWorker1.RunWorkerCompleted
    If Not bClosingForm Then
        If Not e.Cancelled Then
            'Completion Work here
        End If
    End If
End Sub

+1 Handle this at the BackgroundWorker's RunWorkerCompleted EventHandler. That's what I do — , Nov 02 '10 at 19:42

score 2 · Answer 3 · edited Apr 05 '12 at 02:01

I've found another way. If you have more backgroundWorkers you can make:

List<Thread> bgWorkersThreads  = new List<Thread>();

and in every backgroundWorker's DoWork method make:

bgWorkesThreads.Add(Thread.CurrentThread);

Arter that you can use:

foreach (Thread thread in this.bgWorkersThreads) 
{
     thread.Abort();    
}

I used this in Word Add-in in Control, which i use in CustomTaskPane. If someone close the document or application earlier then all my backgroundWorkes finishes their work, it raises some COM Exception(I don't remember exatly which).CancelAsync() doesn't work.

But with this, I can close all threads which are used by backgroundworkers Immediately in DocumentBeforeClose event and my problem is solved.

score 1 · Answer 4 · answered Nov 13 '09 at 21:01

Can you not wait on the signal in the destructor of the form?

AutoResetEvent workerDone = new AutoResetEvent();

private void HandleClosingEvent(object sender, CancelEventArgs e)
{
    this.bgWorker.CancelAsync();
}

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
    while (!this.bgWorker.CancellationPending) {
        Invoke((Action) (() => { this.textBox1.Text =   
                                 Environment.TickCount.ToString(); }));
    }
}


private ~Form1()
{
    workerDone.WaitOne();
}


void backgroundWorker1_RunWorkerCompleted( Object sender, RunWorkerCompletedEventArgs e )
{
    workerDone.Set();
}

score 1 · Answer 5 · edited May 23 '17 at 11:33

Firstly, the ObjectDisposedException is only one possible pitfall here. Running the OP's code has produced the following InvalidOperationException on a substantial number of occasions:

Invoke or BeginInvoke cannot be called on a control until the window handle has been created.

I suppose this could be amended by starting the worker on the 'Loaded' callback rather than the constructor, but this entire ordeal can be avoided altogether if BackgroundWorker's Progress reporting mechanism is used. The following works well:

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
    while (!this.bgWorker.CancellationPending)
    {
        this.bgWorker.ReportProgress(Environment.TickCount);
        Thread.Sleep(1);
    }
}

private void bgWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    this.textBox1.Text = e.ProgressPercentage.ToString();
}

I kind of hijacked the percentage parameter but one can use the other overload to pass any parameter.

It is interesting to note that removing the above sleep call clogs the UI, consumes high CPU and continually increases the memory use. I guess it has something to do with the message queue of the GUI being overloaded. However, with the sleep call intact, the CPU usage is virtually 0 and the memory usage seems fine, too. To be prudent, perhaps a higher value than 1 ms should be used? An expert opinion here would be appreciated... Update: It appears that as long as the update isn't too frequent, it should be OK: Link

In any case, I can't foresee a scenario where the updating of the GUI has to be in intervals shorter than a couple of milliseconds (at least, in scenarios where a human is watching the GUI), so I think most of the time progress reporting would be the right choice

score 1 · Answer 6 · answered May 18 '18 at 23:57

1

I really dont see why DoEvents is regarded as such a bad choice in this case if you are using this.enabled = false. I think it would make it quite neat.

protected override void OnFormClosing(FormClosingEventArgs e) {

    this.Enabled = false;   // or this.Hide()
    e.Cancel = true;
    backgroundWorker1.CancelAsync();  

    while (backgroundWorker1.IsBusy) {

        Application.DoEvents();

    }

    e.cancel = false;
    base.OnFormClosing(e);

}

answered May 18 '18 at 23:57

Peter Jamsmenson

121
1
4

Adding DoEvents() inside my Do...While(IsBusy()) check loop works perfect. The running loop inside my background worker (including the check for CancellationPending is very fast (.0004 mSec) ). Not sure if this is what makes it reliable here. DoEvents() is so universally maligned and anathema to good coding that I completely forgot it existed!! Thanks so much for suggesting it! – Michael Gorsich Nov 06 '20 at 04:45

score 0 · Answer 7 · answered Apr 18 '16 at 13:45

Your backgroundworker should not use Invoke to update the textbox. It should ask the UI thread nicely to update the textbox using event ProgressChanged with the value to put in the textbox attached.

During event Closed (or maybe event Closing), the UI thread remembers that the form is closed before it cancels the backgroundworker.

Upon receiving the progressChanged the UI thread checks if the form is closed and only if not, it updates the textbox.

score 0 · Answer 8 · answered Dec 18 '16 at 12:09

This won't work for everyone, but if you are doing something in a BackgroundWorker periodically, like every second or every 10 seconds, (perhaps polling a server) this seems to work well to stop the process in an orderly manner and without error messages (at least so far) and is easy to follow;

 public void StopPoll()
        {
            MyBackgroundWorker.CancelAsync(); //Cancel background worker
            AutoResetEvent1.Set(); //Release delay so cancellation occurs soon
        }

 private void bw_DoWork(object sender, DoWorkEventArgs e)
        {
            while (!MyBackgroundWorker.CancellationPending)
            {
            //Do some background stuff
            MyBackgroundWorker.ReportProgress(0, (object)SomeData);
            AutoResetEvent1.WaitOne(10000);
            }
    }

score -1 · Answer 9 · answered Nov 13 '09 at 19:48

-1

I'd pass in the SynchronizationContext associated with the textbox to the BackgroundWorker and use that to perform Updates on the UI thread. Using SynchronizationContext.Post, you can check if the control is disposed or disposing.

answered Nov 13 '09 at 19:48

Hasani Blackwell

2,026
1
13
10

`WindowsFormsSynchronizationContext.Post(...)` just calls `BeginInvoke(...)`, so it isn't much different from Invoke() I'm doing already. Unless I miss something, could you please elaborate? – THX-1138 Nov 13 '09 at 20:07

score -1 · Answer 10 · answered Aug 01 '13 at 10:48

What about Me.IsHandleCreated?

    Private Sub BwDownload_RunWorkerCompleted(sender As Object, e As System.ComponentModel.RunWorkerCompletedEventArgs) Handles BwDownload.RunWorkerCompleted
    If Me.IsHandleCreated Then
        'Form is still open, so proceed
    End If
End Sub

score -2 · Answer 11 · answered Apr 04 '12 at 18:17

-2

Another way:

if (backgroundWorker.IsBusy)
{
    backgroundWorker.CancelAsync();
    while (backgroundWorker.IsBusy)
    {
        Application.DoEvents();
    }
}

answered Apr 04 '12 at 18:17

Kergorian

11

score -2 · Answer 12 · edited Apr 05 '12 at 02:03

One solution that works, but too complicated. The idea is to spawn the timer that will keep trying to close the form, and form will refuse to close until said bgWorker is dead.

private void HandleClosingEvent(object sender, CancelEventArgs e) {
    if (!this.bgWorker.IsBusy) {
        // bgWorker is dead, let Closing event proceed.
        e.Cancel = false;
        return;
    }
    if (!this.bgWorker.CancellationPending) {
        // it is first call to Closing, cancel the bgWorker.
        this.bgWorker.CancelAsync();
        this.timer1.Enabled = true;
    }
    // either this is first attempt to close the form, or bgWorker isn't dead.
    e.Cancel = true;
}

private void timer1_Tick(object sender, EventArgs e) {
    Trace.WriteLine("Trying to close...");
    Close();
}

this sounds very hacky to me. won't do this. – Marc Jun 19 '12 at 06:26 — Marc, Jun 19 '12 at 06:26

How to stop BackgroundWorker on Form's Closing event?

12 Answers12

Linked

Related