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
    }

Answer 1

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();
    }

Answer 2

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.

Answer 3

I personally would run treat the thread as the model and the form as the view/controller.

Instead of the thread changing the controls, I would have the form change it's own controls based on either events raised by the thread or via some sort of update timer. Furthermore, if I wanted the user to stop or start the thread, I'd implement from methods in the thread class that the form could call.

By trying to create and destroy the thread using code in the form, you're running into a host of issues. I suggest taking a look at the Model/View/Controller design pattern.

Answer 4

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();
}

Answer 5

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.

    protected override void  OnFormClosing(FormClosingEventArgs e) {
        if (!mCompleted) {
            backgroundWorker1.CancelAsync();
            this.Enabled = false;
            e.Cancel = true;
            mClosePending = true;
        }
    }
    void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e) {
        mCompleted = true;
        if (mClosePending) this.Close();
    }

Answer 6

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

Answer 7

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 chancel 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, but I did add in 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