Cross-Thread Operations and the use of Invoke repetitive times, best approach

Question

This is a WinForms application with a class that have several methods taking lot of time, those methods raise events at certain interval that I use to update the GUI, change labels, texts, colors, and so forth.

The Tasks run on a different thread, so they are not blocking the UI, that is fine, since I either call them from a System.Timers.Timer or create a list of Tasks and run them async.

The problem I am having is that, on the event handler I created for that class that reports me different status I cannot update the GUI, because of a cross-thread operation, so what I did to solve this is check if InvokeRequired on the EventHandler and then BeginInvoke.

That did the trick, but I'm sure this is not right, my EventHandler can be called hundred of times and each time I begin a new Invoke. What is the best approach to do? I am not experienced with Delegates and somehow I believe I should be using them, not sure how

I leave here a sample piece of code that pretty much simplifies the workload and issue

public partial class Form1 : Form {
  private Operational o = new();
  private System.Timers.Timer WorkerTimer=new() { Interval=1000,Enabled=false};

  public Form1() {
    InitializeComponent();
    o.OperationComplete += OperationCompleteEventHandler;
    WorkerTimer.Elapsed += MonitorExecutor;
  }

  private async void MonitorExecutor(object? sender, ElapsedEventArgs e) {
    List<Task> myTasks = new();
    myTasks.Add(Task.Run(o.DoWork));
    myTasks.Add(Task.Run(o.DoWork));
    await Task.WhenAll(myTasks);
    // Report all tasks have completed
  }

  private void OperationCompleteEventHandler(object? sender, EventArgs e) {
    if (InvokeRequired) {
      // Otherwise it will throw a cross-thread exception
      Debug.WriteLine("Invoked Required!");
      BeginInvoke(() => OperationCompleteEventHandler(sender, e));
      return;
    }
    label1.Text = "WorkCompleted";
    // But this could also take a lot of time,
    // so I don't want this method to hang my thread
    Thread.Sleep(500);
  }

  private void button1_Click(object sender, EventArgs e) {
    WorkerTimer.Enabled = !WorkerTimer.Enabled;
    button1.Text = WorkerTimer.Enabled ? "Running" : "Stopped";
  } 
}

public class Operational {
  public event EventHandler? OperationComplete;

  public void DoWork() {
    // Long Operations
    Thread.Sleep(500);
    OperationComplete?.Invoke(this, new());
  }

}

score 1 · Accepted Answer · answered Jun 22 '23 at 23:40

You are not using await quite right. Given that WinForms sets up a SynchronizationContext, you should rely on that to marshal continuations (the bit after the await) onto the correct thread.

There are a number of different ways to do this, but primarily you need to move the invocation of OperationComplete to a normal await, and run the rest of the code using Task.Run.

public async Task DoWork()
{
    await Task.Run(DoWorkCore);
    OperationComplete?.Invoke(this, new());
}

private void DoWorkCore()
{
    // Long Operations
    Thread.Sleep(500);
}

Then you can remove the whole if (InvokeRequired) { block, because now OperationCompleteEventHandler will always run on the UI thread.

private async void OperationCompleteEventHandler(object? sender, EventArgs e)
{
    label1.Text = "WorkCompleted";
    await Task.Run(() => {
        // some other long running stuff
        Thread.Sleep(500);
    });
    // more UI stuff here
}

And MonitorExecutor can simply be

private async void MonitorExecutor(object? sender, ElapsedEventArgs e)
{
    List<Task> myTasks = new();
    myTasks.Add(o.DoWork);
    myTasks.Add(o.DoWork);
    await Task.WhenAll(myTasks);
    // Report all tasks have completed
}

If you don't want to change DoWork then you will need to use some kind of Event-to-Task conversion such as A reusable pattern to convert event into task or General purpose FromEvent method

Theodor Zoulias · Answer 2 · 2023-06-23T01:13:45.847

I would advise against using the System.Timers.Timer component. It's not a well designed component IMHO. You can read my arguments at the bottom of this answer.

For doing work periodically without the risk of overlapping executions, you could use an async method and the PeriodicTimer component. For a usage example see this answer.

As for invoking the UI thread hundreds of times per second, it's not a good idea because of the risk of overburdening the UI thread, making your application non responsive, especially if it runs on slow machines. You should reduce the frequency of updating the UI to 10-20 updates per second at most. Updating the UI more frequently than this is meaningless, because it's beyond the visual capabilities of humans at processing changing information. For throttling the flow of information you could use tools like the Rx with the Debounce/Throttle operator, or achieve the same effect with custom code that uses Stopwatches and TimeSpans, or use an incremented counter and update the UI only when ++counter % 10 == 0 etc.

score 1 · Answer 3 · answered Jun 23 '23 at 03:09

Just in case you're open to alternative ways of doing this, here is an option using Microsoft's Reactive Framework (aka Rx) - NuGet System.Reactive.Windows.Forms and add using System.Reactive.Linq; - then you can do this:

public partial class Form1 : Form
{
    private Operational o = new();

    public Form1()
    {
        InitializeComponent();
        Observable
            .Interval(TimeSpan.FromSeconds(2.0))
            .ObserveOn(this)
            .Do(x => label1.Text = "WorkStarted")
            .SelectMany(x => Observable.FromAsync(() => o.DoWork()))
            .SelectMany(x => Observable.FromAsync(() => o.DoWork()))
            .ObserveOn(this)
            .Subscribe(x => label1.Text = "WorkCompleted");
    }
}

public class Operational
{
    public async Task DoWork()
    {
        await Task.Delay(TimeSpan.FromMilliseconds(250.0));
    }
}

It's perhaps a little more concise and potentially easier to reason about.

Cross-Thread Operations and the use of Invoke repetitive times, best approach

3 Answers3