Cleaning up code littered with InvokeRequired [duplicate]

Question

I know that when manipulating UI controls from any non-UI thread, you must marshal your calls to the UI thread to avoid issues. The general consensus is that you should use test InvokeRequired, and if true, use .Invoke to perform the marshaling.

This leads to a lot of code that looks like this:

private void UpdateSummary(string text)
{
    if (this.InvokeRequired)
    {
        this.Invoke(new Action(() => UpdateSummary(text)));
    }
    else
    {
        summary.Text = text;
    }
}

My question is this: can I leave out the InvokeRequired test and just call Invoke, like so:

private void UpdateSummary(string text)
{
    this.Invoke(new Action(() => summary.Text = text));
}

Is there a problem with doing this? If so, is there a better way to keep the InvokeRequired test while not having to copy and paste this pattern all over the place?

Histoid

It should be pretty doggone rare that you have code that's used by a thread that could also be executed on the UI thread. Yeah, don't bother testing InvokeRequired when you got lambdas at your disposal. – Deportee

@Hans - I don't know, I work in control systems and I have code which does this. I often have a number of threads running async process operations, any of which, in addition to the UI thread, can make calls to shared logging, indicator updates, or calls to halt the global process. Each thread handles a different aspect of the process but they also have to have access to certain core methods, which they call through Invoke. Since a local operator must also have access to those many of those methods via the UI, they end up sharing a lot of code. – Convene

InvokeRequired is only necessary for UI, don't use it for anything else. Shared logging only requires a lock. In soft real-time programs, hiding the context switch in a helper method is a fairly dangerous anti-pattern. – Deportee

Logging to a file, yes, but when also logging to screen or adding items to a list box then it becomes a UI issue. Many of these actions are all part of whole procedures, as well. While it may do to have worker threads churn through them and invoke line-by-line as needed most of the time it is easier to just let the UI thread deal with them - let the workers delegate the whole thing and get on with their more time-critical operations. Point being, it is not an impossible scenario. – Convene

That's pretty much the boat I was in. I ended up not using the helper I 'accepted' on, and just kept the InvokeRequired tests. – Histoid

stackoverflow.com/a/10209287 is a valid point. A control's handle should be created for Invoke to work. To be safe just check for InvokeRequired always. – Hiro

Answer 1

Well how about this:

public static class ControlHelpers
{
    public static void InvokeIfRequired<T>(this T control, Action<T> action) where T : ISynchronizeInvoke
    {
        if (control.InvokeRequired)
        {
            control.Invoke(new Action(() => action(control)), null);
        }
        else
        {
            action(control);
        }
    }
}

Use it like this:

private void UpdateSummary(string text)
{
    summary.InvokeIfRequired(s => { s.Text = text });
}

Epitomize

I've done exactly the same thing except I also added public static T InvokeIfRequired(this Control control, Func<T> function) as well for when I want to return a value. – Association

This pattern can be used in other situations aswell. At my company, we use Funs.CatchLog(Action a, string name) to catch and log "forgotten" exceptions. Funs.Measure(Action a, string name) measures how much time was spent in an action and so on... – Symphonious

Answer 2

Calling Invoke from the UI thread is somewhat inefficient.

Instead, you can create an InvokeIfNeeded extension method that takes an Action parameter. (this would also allow you to remove new Action(...) from the callsite)

Macmullin

@Gabe: It interacts with the message queue. – Macmullin

SLaks: Invoke is synchronous, so wouldn't it deadlock if it tried to interact with the message queue on the UI thread? – Tarbes

@Gabe: No. It checks whether it's on the UI thread, and, if so, synchronously empties the queue. (In other words, it's a reentrant call) – Macmullin

SLaks: You're saying that Invoke causes your action to get executed after any other queued invokes, as opposed to before the other queued invokes. How does changing the order make it inefficient? – Tarbes

@Gabe: Not calling Invoke will not run any queued invokes. Also, the queue probably involves locks. – Macmullin

SLaks: But those queued invokes would run on the UI thread anyway, just a bit later. – Tarbes

Answer 3

I've been reading about the arguments back and forth on adding a logic check to find out if the invoke should be used IFF when not on the UI thread and not on the UI thread itself. I wrote a class that examines the time to execute (via Stopwatch) of various methods to get a rough estimate of the efficiency of one method over another.

The results may be surprising to some of you (these tests were run via the Form.Shown event):

     // notice that we are updating the form's title bar 10,000 times
     // directly on the UI thread
     TimedAction.Go
     (
        "Direct on UI Thread",
        () =>
        {
           for (int i = 0; i < 10000; i++)
           {
              this.Text = "1234567890";
           }
        }
     );

     // notice that we are invoking the update of the title bar
     // (UI thread -> [invoke] -> UI thread)
     TimedAction.Go
     (
        "Invoke on UI Thread",
        () =>
        {
           this.Invoke
           (
              new Action
              (
                 () =>
                 {
                    for (int i = 0; i < 10000; i++)
                    {
                       this.Text = "1234567890";
                    }
                 }
              )
           );
        }
     );

     // the following is invoking each UPDATE on the UI thread from the UI thread
     // (10,000 invokes)
     TimedAction.Go
     (
        "Separate Invoke on UI Thread",
        () =>
        {
           for (int i = 0; i < 10000; i++)
           {
              this.Invoke
              (
                 new Action
                 (
                    () =>
                    {
                       this.Text = "1234567890";
                    }
                 )
              );
           }
        }
     );

Results are as follows:

• TimedAction::Go()+0 - Debug: [DEBUG] Stopwatch [Direct on UI Thread]: 300ms
• TimedAction::Go()+0 - Debug: [DEBUG] Stopwatch [Invoke on UI Thread]: 299ms
• TimedAction::Go()+0 - Debug: [DEBUG] Stopwatch [Separate Invoke on UI Thread]: 649ms

My conclusion is that you can safely invoke at any time, regardless whether you are on the UI thread or a worker thread, without significant overhead of looping back via the message pump. However, performing most of the work ON the UI thread instead of making many calls to the UI thread (via Invoke()) is advantageous and improve efficiency greatly.

Cassey

...so invoke costs about 35 microseconds on whatever platform you are using. I suppose for some people that is enormous and for many others it is negligible. Nice experiment. – Convene

Answer 4

I realize that there's already one answer that's pretty much spot on, but I wanted to also post my take on it (which I also posted here).

Mine is a little different in that it can slightly more safely handle null controls and can return results when necessary. Both of these have come in handy for me when trying to Invoke showing a MessageBox on a parent form that might be null, and returning the DialogResult of showing that MessageBox.

---

using System;
using System.Windows.Forms;

/// <summary>
/// Extension methods acting on Control objects.
/// </summary>
internal static class ControlExtensionMethods
{
    /// <summary>
    /// Invokes the given action on the given control's UI thread, if invocation is needed.
    /// </summary>
    /// <param name="control">Control on whose UI thread to possibly invoke.</param>
    /// <param name="action">Action to be invoked on the given control.</param>
    public static void MaybeInvoke(this Control control, Action action)
    {
        if (control != null && control.InvokeRequired)
        {
            control.Invoke(action);
        }
        else
        {
            action();
        }
    }

    /// <summary>
    /// Maybe Invoke a Func that returns a value.
    /// </summary>
    /// <typeparam name="T">Return type of func.</typeparam>
    /// <param name="control">Control on which to maybe invoke.</param>
    /// <param name="func">Function returning a value, to invoke.</param>
    /// <returns>The result of the call to func.</returns>
    public static T MaybeInvoke<T>(this Control control, Func<T> func)
    {
        if (control != null && control.InvokeRequired)
        {
            return (T)(control.Invoke(func));
        }
        else
        {
            return func();
        }
    }
}

Usage:

myForm.MaybeInvoke(() => this.Text = "Hello world");

// Sometimes the control might be null, but that's okay.
var dialogResult = this.Parent.MaybeInvoke(() => MessageBox.Show(this, "Yes or no?", "Choice", MessageBoxButtons.YesNo));

Assignat

I came up with a method similar to your second one before I saw this answer. In my case, I needed this so I could invoke a ListViewItem collection property and use Linq to read & transform its items. – Firstclass

Answer 5

I am not convinced that Control.Invoke is the best choice for updating the UI. I cannot say for sure in your case because I do not know the circumstances under in which UpdateSummary in called. However, if you are calling it periodically as a mechanism for displaying progress information (which is the impression I get from the code snippet) then there is usually a better option. That option is to have the UI thread poll for the status instead of having the worker thread push it.

The reasons why the polling approach should be considered in this case is because:

• It breaks the tight coupling between the UI and worker threads that Control.Invoke imposes.
• It puts the responsibility of updating the UI thread on the UI thread where it should belong anyway.
• The UI thread gets to dictate when and how often the update should take place.
• There is no risk of the UI message pump being overrun as would be the case with the marshaling techniques initiated by the worker thread.
• The worker thread does not have to wait for an acknowledgement that the update was performed before proceeding with its next steps (ie. you get more throughput on both the UI and worker threads).

So consider creating a System.Windows.Forms.Timer that periodically checks for the text to be displayed on the Control instead of initiating the push from the worker thread. Again, without knowing your exact requirements I am not willing to say this definitely the direction you need to go, but in most many cases it is better than the Control.Invoke option.

Obviously this approach eliminates the necessity of the InvokedRequired check entirely. Nevermind, the fact that it simplies all other aspects of the UI / worker thread interaction.

Morbilli

I don't think that this 'simplies all aspects of the UI / worker thread interaction'. In fact, now you have to add synchronization around the GetCurrentStatus function on the worker. So, depending on how complex the worker already is, you could be introducing a wide array of threading issues. – Epitomize

@John: Maybe, though the general pattern is to package the progress/status information in an immutable class and publish it by writing to a shared variable as a single point of interation. The only thing required here would be the volatile keyword. That seems a lot simplier than Control.Invokeing in multiple places and having to consider the impacts it causes on both threads. I did edit my answer to use a less imposing tone regarding that point though. – Morbilli

Answer 6

My preferred approach for view-only controls is to have all of the control state encapsulated in a class which can be updated without ever going through any inconsistent states (a simple way to do this is to put all things that need to be updated together into an immutable class, and create a new instance of the class whenever an update is needed). Then have a method which will Interlocked.Exchange an updateNeeded flag and, if there isn't an update pending but IsHandleCreated is true, then BeginInvoke the update procedure. The update procedure should clear the updateNeeded flag as the first thing it does, before doing any updates (if someone tries to update the control at that point, another request will be BeginInvoked). Note that you must be prepared to catch and swallow an exception (I think IllegalOperation) if the control gets disposed just as you're preparing to update it.

Incidentally, if a control hasn't yet been joined to a thread (by being added to a visible window, or having the window it's on become visible), it's legal to update it directly but not legal to use BeginInvoke or Invoke on it.

Confectionary

Answer 7

I cannot comment yet, hopefully someone will see this and add it to the accepted answer, which otherwise is spot on.

control.Invoke(new Action(() => action(control))); should read
control.Invoke(new Action(() => action(control)), null);

As written the accepted answer will not compile because ISynchronizeInvoke.Invoke() does not have an overload with only 1 argument like Control.Invoke() does.

One other thing is that the usage might be more clear as
summary.InvokeIfRequired(c => { summary.Text = text; }); rather than as written summary.InvokeIfRequired(c => { textBox.Text = text });

Choroid

Answer 8

It is easier to use BackgroudWorker, if possible, for making the UI responsive and use ReportProgress to update the UI because it runs on the same thread as the UI, hence you will not need InvokeRequired.

Dav

I can't use BackgroundWorker in this case as I require pretty fine-grained control over the threads themselves. Thanks though =) – Histoid

Not always possible to do so. ReportProgress can get messy as well if you are trying to do more than literally report progress. – Knacker