Should Control.InvokeRequired always be used when accessing UI controls

Question

We are building a .NET application using WinForms (3.5).

I have added a new feature recently, and started experiencing weird behavior when accessing certain controls. The problem was that some UI control access simply halted the execution (no exception was seen).

Under close examination (using WinDbg) I realized the controls were being updated from a ThreadPool thread, and a CrossThreadMessagingException was thrown.

My question is -- is there any good practice on how to circumvent such behavior?

It would be very cumbersome but also perhaps not possible to surround every code location that accesses UI controls with the Control.Invoke method.

How can i partition my code to "safe" code that shouldn't use Invoke, from one that should?

Cur

Safe code runs on the UI thread. All other code that touches the UI must use the Invoke route. – Iconolatry

I don't always control where the code runs... Some code may get called from within methods that are running on the threadpool threads, etc. That is why i am looking for a pattern or best practice to avoid these scenarios (instead of off course finding them during testing). – Cur

Code that "may get called from the threadpool" shouldn't access the GUI. And when it (very rarely) really has to, surround it with InvokeRequired. – Bellay

Yeah, you're going to need to stop calling UI methods from non UI threads. You sound as though you are in denial over that. – Koppel

You said "no exception was seen": It might have to do with a misbehaviour of Windows blog.paulbetts.org/index.php/2010/07/20/… – Freese

I am not in denial. Howver, lets say i have an event handler that calls MethodA. This handler is raised on the UI thread. In other code parts some other code calls MethodA. Of course there are more levels to this (method a calls b that calls .... Eventually something touches the UI). Back to my question then. HOW can i easily manage such scenarios so i won't run into them – Cur

Answer 1

If the application is designed to be multithreaded, then cross-threading can occur and thus you need to check for it using InvokeRequired, and either have the method you are calling re-Invoke() itself on the UI thread, or throw an exception that will indicate code is being used improperly. Keep in mind that InvokeRequired will be false in certain circumstances (mainly when the window doesn't have a handle or is being/has been disposed); the best way to prevent these circumstances is to not start threads earlier in the window initialization process than the Load() event handler, and to handle the Closing() event by cancelling background threads created by the window and waiting for them to close.

If the application is not multithreaded (you are not setting up BackgroundWorkers, TPL operations, BeginInvoke()ing delegates or Start()ing threads), then it's not necessary. However, calls to InvokeRequired are pretty cheap (the logic behind it is basically a check that the WinAPI functions GetThreadId and GetWindowThreadProcessId return the same value), so if you anticipate the program being restructured to be multithreaded, the following patterns for called methods are simple enough to implement:

//no return value, no parameters; ShowWindow(), HideWindow(), etc
//Understand that many built-in control methods are not virtual and so you can't 
//override them to do this; you must either hide them or ensure the caller is
//checking for cross-threading.
public void MyWindowMethod()
{
   if(InvokeRequired)
      this.Invoke(new Action(MyWindowMethod));
   else
   {
      //main logic
   }
}

//Input but no return; SetTitle("My Title")
public void MyWindowMethod2(string input)
{
   if(InvokeRequired)
      this.Invoke(new Action<string>(MyWindowMethod2), input);
   else
   {
      //main logic
   }
}

//inputs and outputs; custom methods, advanced graphics
public string MyWindowMethod3(string input)
{
   if(InvokeRequired)
      return (string)(this.Invoke(new Func<string, string>(MyWindowMethod3), input));

   //No else required; the return makes it redundant
   //main logic   
}

Degraw