khargoosh
khargoosh

Reputation: 1520

Safe to Invoke within Form.Dispose() method

I have an ApplicationContext with a Form. Various asynchronous communication and timing threads may occassionally have a need to restart the application, however before I can restart I have to manually dispose of MyApplicationContext which needs to free resources that will be needed immediately when the new process starts.

It seems that just calling Application.Restart() does not dispose of the resources quickly enough in this case.

Within the call to MyApplicationContext.Dispose() the subsequent call to base.Dispose(disposing) eventually calls the Form.Dispose() method, and because this can originate from various threads I have seen a cross-thread operation exception occur.

/// MyApplicationContext.requestRestart()
private void requestRestart()
{
    this.Dispose(); // dispose of applicationcontext
    Application.Restart();
}

leads to...

/// MyApplicationContext.Dispose(bool)
protected override void Dispose(bool disposing) 
{
    /// dispose stuff
    base.Dispose(disposing);
}

leads to...

/// MainForm.Dispose(bool)
protected override void Dispose(bool disposing)
{
    /// dispose stuff
    base.Dispose(disposing);
}

Which can be called from any thread.

Is it safe to Invoke the overridden dispose handler on the Form UI thread like this?

protected override void Dispose(bool disposing)
{
    if (this.InvokeRequired)
    {
        this.Invoke(new Action(() => Dispose(disposing)));
    }
    else
    {
        if (disposing && (components != null))
        {
            components.Dispose();
        }
        base.Dispose(disposing);
    }
}

Upvotes: 3

Views: 1323

Answers (1)

Peter Duniho
Peter Duniho

Reputation: 70671

No, it is not safe to call Invoke() from a Dispose() implementation.

Okay, now that I've said that: sure, you could probably get away with it. But it's a really bad idea, and it really could lead to real problems.

The Dispose() method is supposed to be simple. It should not be accessing managed objects, and it should complete quickly. Calling Invoke() can lead to indeterminate delays, depending on what else is going on in the program, and even outright deadlock.

Beyond that, an important rule in the Dispose() method is to not access managed objects. At the very least, your implementation needs to check disposing first, to ensure that you don't attempt to call Invoke() when being called from the finalizer. Furthermore, it's definitely also not a good idea to dispose the object that is currently being used to call the Invoke() method. If you really must use cross-thread invocation, the right way to do it would be to retrieve the SynchronizationContext for the current object and use that instead of calling Control.Invoke().


But really, the right thing to do here is to fix your restart logic so the entire disposing operation occurs in the UI thread that owns the objects that are being disposed. Your objects aren't the ones that should be responsible to moving execution to the correct thread; the code that uses those object should be instead. By the time your Form class's code is involved, you should already have made sure the code's executing in the UI thread that owns that object.

Upvotes: 3

Related Questions