Updating controls in Main Thread using EventHandler

Question

I'm using this to update a control in the main thread from another thread:

private void ShowHourGlassSafe(bool visible)
{
    this.Invoke((EventHandler)((s, ev) => pictureBoxHourGlass.Visible = visible));           
}

I wonder what are the implications of doing it this way or if is there any risk this is going to fail?

From the many examples are there for the same thing I could not find one like this.

It could be that it's simply wrong?

Answer 1

Typical problems with this kind of code:

• You'll deadlock if the UI thread is doing something unwise like waiting for the thread to complete. There's no point in using Invoke, that blocks the worker thread for no benefit, just use BeginInvoke. Solves the deadlock potential and the unnecessary delay.

• You'll crash when the UI was closed and pictureBoxHourGlass was disposed. Ensuring that the thread is no longer running before allowing the UI to close is very commonly overlooked. Just displaying an hour glass isn't enough, you also have to take countermeasures to prevent the user from closing the UI. Or otherwise interlock it with a way to cancel the thread first

• The user will typically be befuddled when an hour glass shows up without him doing anything to ask that something gets done. The 99% correct case is that you display the hour glass with code in the UI thread and then start the thread. And hide it again when the thread completes. Easiest to do with the BackgroundWorker or Task classes, they can run code on the UI thread after the job was done.

Favor the Action delegate types for consistency:

    private void ShowHourGlassSafe(bool visible) {
        this.BeginInvoke(new Action(() => something.Visible = visible));
    }

Answer 2

Well, you've picked a rather odd delegate to choose, as you've chosen one that has two parameters despite the fact that none are needed nor will be provided. I don't know if that will cause it to break, but it's certainly doing nothing to help. You're most likely best off using a delegate that takes no parameters and returns no values, such as:

private void ShowHourGlassSafe(bool visible)
{
    this.Invoke((MethodInvoker)(() => pictureBoxHourGlass.Visible = visible));           
}

Other than that, the fundamental concept of what you're doing is perfectly fine.