How to correctly dispose a Form, without risk of an Invoke being called from another thread on a disposed object?

Question

I have a Form which "listens" to events that are raised elsewhere (not on the Form itself, nor one of its child controls). Events are raised by objects which exist even after the Form is disposed, and may be raised in threads other than the one on which the Form handle was created, meaning I need to do an Invoke in the event handler (to show the change on the form, for example).

In the Dispose(bool) method of the form (overridden) I unsubscribed from all events that may still be subscribed when this method is called. However, Invoke is still called sometimes from one of the event handlers. I assume this is because the event handler gets called just a moment before the event is unsubscribed, then OS switches control to the dispose method which executes, and then returns control back to the handler which calls the Invoke method on a disposed object.

Locking the threads doesn't help because a call to Invoke will lock the calling thread until main thread processes the invoked method. This may never happen, because the main thread itself may be waiting for a release of the lock on the object that the Invoke-calling thread has taken, thus creating a deadlock.

So, in short, how do I correctly dispose of a Form, when it is subscribed to external events, which may be raised in different threads?

Here's how some key methods look at the moment. This approach is suffering the problems I described above, but I'm not sure how to correct them.

This is an event handler handling a change of Data part of the model:

private void updateData()
{
 if (model != null && model.Data != null)
 {
  model.Data.SomeDataChanged -= new MyEventHandler(updateSomeData);

  model.Data.SomeDataChanged += new MyEventHandler(updateSomeData);
 }
 updateSomeData();
}

This is an event handler which must make changes to the view:

private void updateSomeData()
{
 if (this.InvokeRequired) this.myInvoke(new MethodInvoker(updateSomeData));
 else
 {
  // do the necessary changes
 }
}

And the myInvoke method:

private object myInvoke(Delegate method)
{
 object res = null;
 lock (lockObject)
 {
  if (!this.IsDisposed) res = this.Invoke(method);
 }
 return res;
}

My override of the Dispose(bool) method:

protected override void Dispose(bool disposing)
{
 lock (lockObject)
 {
  if (disposing)
  {
   if (model != null)
   {
    if (model.Data != null)
    {
     model.Data.SomeDataChanged -= new MyEventHandler(updateSomeData);
    }
    // unsubscribe other events, omitted for brevity
   }
   if (components != null)
   {
    components.Dispose();
   }
  }
  base.Dispose(disposing);
 }
}

Update (as per Alan's request):

I never explicitly call the Dispose method, I let that be done by the framework. The deadlock has so far only happened when the application is closed. Before I did the locking I sometimes got some exceptions thrown when a form was simply closed.

Answer 1

I had the problem with the Invoke method while multithreading, and I found a solution that works like a charm!

I wanted to create a loop in a task that update a label on a form to do monitoring.

But when I closed the form window, my Invoke threw an exception because my Form is disposed !

Here is the pattern I implemented to resolve this problem:

class yourClass : Form
{
    private bool isDisposed = false;
    private CancellationTokenSource cts;
    private bool stopTaskSignal = false;
    public yourClass()
    {
        InitializeComponent();
        this.FormClosing += (s, a) =>
        {
            cts.Cancel();
            isDisposed = true;
            if (!stopTaskSignal)
                a.Cancel = true;
        };
    }

    private void yourClass_Load(object sender, EventArgs e)
    {
        cts = new CancellationTokenSource();
        CancellationToken token = cts.Token;

        Task.Factory.StartNew(() =>
        {
            try
            {
                while (true)
                {
                    if (token.IsCancellationRequested)
                    {
                        token.ThrowIfCancellationRequested();
                    }

                    if (this.InvokeRequired)
                    {
                        this.Invoke((MethodInvoker)delegate { methodToInvoke(); });
                    }
                }
            }
            catch (OperationCanceledException ex)
            {
                this.Invoke((MethodInvoker)delegate { stopTaskSignalAndDispose(); });
            }
        }, token);
    }

    public void stopTaskSignalAndDispose()
    {
        stopTaskSignal = true;
        this.Dispose();
    }

    public void methodToInvoke()
    {
        if (isDisposed) return;
        label_in_form.Text = "text";
    }
}

I execute methodToInvoke() in an invoke to update the label from the form's thread.

When I close the window, the FormClosing event is called. I take this opportunity to cancel the closing of the window (a.Cancel) and to call the Cancel method of the object Task to stop the thread.

I then access the ThrowIfCancellationRequested() method which throws an OperationCanceledException allowing, juste after, to exit the loop and complete the task.

The Invoke method sends a "Window message" in a Queue.

Microsoft says : « For each thread that creates a window, the operating system creates a queue for window messages. »

So I call another method that will now really close the window but this time by using the Invoke method to make sure that this message will be the last of the Queue!

And then I close the window with the Dispose() method.

Answer 2

I'd like to provide a sort of addendum to supercat's answer that may be interesting.

Begin by making a CountdownEvent (we'll call it _invoke_counter) with an initial count of 1. This should be a member variable of the form (or control) itself:

private readonly CountdownEvent _invoke_counter = new CountdownEvent(1);

Wrap each use of Invoke/BeginInvoke as follows:

if(_invoke_counter.TryAddCount())
{
    try
    {
        //code using Invoke/BeginInvoke goes here
    }
    finally { _invoke_counter.Signal(); }
}

Then in your Dispose you can do:

_invoke_counter.Signal();
_invoke_counter.Wait();

This also allows you to do a few other nice things. The CountdownEvent.Wait() function has an overload with a timeout. Perhaps you only want to wait a certain period of time to let the invoking functions finish before letting them die. You could also do something like Wait(100) in a loop with a DoEvents() to keep things responsive if you expect the Invokes to take a long time to finish. There's a lot of niftyness you can achieve with this method.

This should prevent any weird timing race condition type of issues and it's fairly simple to understand and implement. If anyone sees any glaring problems with this, I'd love to hear about them because I use this method in production software.

IMPORTANT: Make sure that the disposal code is on the Finalizer's thread (which it should be in a "natural" disposal). If you try to manually call the Dispose() method from the UI thread, it will deadlock because it will get stuck on the _invoke_counter.Wait(); and the Invokes won't run, etc.

Answer 3

There are two approaches to consider. One is to have a locking object within the Form, and have the internal calls to Dispose and BeginInvoke calls occur within the lock; since neither Dispose nor BeginInvoke should take very long, code should never have to wait long for the lock.

The other approach is to just declare that because of design mistakes in Control.BeginInvoke/Form.BeginInvoke, those methods will sometimes throw an exception that cannot practically be prevented and should simply be swallowed in cases where it won't really matter whether or not the action occurs on a form which has been disposed anyway.