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?

11 Answers
11

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.

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 PassantJan 26 '10 at 20:33

2

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

1

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

4

@lain : No, OnFormClosing and backgroundWorker1_RunWorkerCompleted both run on the UI thread. One cannot be interrupted by the other.
–
Sacha KMay 23 '13 at 13:36

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

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:

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

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.

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.

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-1138Nov 13 '09 at 20:07

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.

It's a simplified example. Breaking the system into multiple components doesn't resolve the issue, since closing event still originates by user clicking "X" to close the form.
–
THX-1138Nov 13 '09 at 20:40

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

Please keep a list of Threads in a List threads object, and Then on form close event
You can loop through each element in the thread's list and Abort one by one ,

Or another option is to set your thread's IsBackGround property to true then it will close all the threads when the main thread is closed , IF you are not closing the main thread and just closing the form , then You have to create a list as I mentioned above in this answer.