1

I am writing a client server application that works like this:

Form1 loads and creates ServerHost. ServerHost is started to listen for TcpClient connections, on connected and accepted, ServerHost spawns a thread by way of ThreadPool.QueueUserWorkItem(DoWork, client);

Within the DoWork() thread, I want to update Winform Controls on Form1.

This is achieved by having events in ServerHost such as ServerHost.SomethingHappened. When something happened in DoWork(), it raises the event and the Form1.Handler is called to update the winforms control.

This set up gives me cross-thread operation error.

Is use of Control.Invoke and Control.InvokeRequired healthy? I am not good at threads, and MSDN is saying to use BackgroundWorker, but I can't see how to do it here. Any advice to change the structure to avoid using Invoke in this set up?

Community
  • 1
  • 1
Jake
  • 11,273
  • 21
  • 90
  • 147
  • With rampant use, and especially with many worker threads, BeginInvoke() is a lot less harmful than Invoke(). – H H Mar 12 '14 at 10:01

5 Answers5

3

Control.Invoke is highly questionable, and Control.InvokeRequired is downright toxic.

If at all possible, use the new async/await support, and you won't need to explicitly marshal back to the UI thread. Also, use Task.Run instead of ThreadPool.QueueUserWorkItem for background work.

The problem with Control.Invoke is that it ties your library to a specific UI (WinForms). Capturing a SynchronizationContext is a step above that, and implicitly capturing the SynchronizationContext by using await is even better.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
2

You have to invoke the code that updates the user interface on the UI thread.

In general there are several options to do that:

  • calling Invoke on a Control
  • using a BackgroundWorker that has been started on the UI thread
  • calling Post on the SynchronizationContext of the UI thread
  • using Task.ContinueWith with the TaskScheduler of the UI thread
  • using asynchronous calls with async/await

In my opinion last method is by far the easiest for the developer, but it is only available with C# 5 and .NET 4.5 or .NET 4.0 with the Microsoft.Bcl.Async package. Tasks are nearly as easy to use but both of these methods would require you to change your code. They won't work to simply invoke a method on the UI thread from a thread pool thread.

The BackgroundWorker is usually used to schedule an action that takes quite some time. Its ReportProgress method raises the ProgressChanged event on the thread that called the RunWorkerAsync method. As such it is also not a good solution to your problem.

SynchronizationContext.Post and Control.Invoke work similarly, but Control.Invoke doesn't require you to capture the UI context, so it's easier to use.

To summarize it you should use Control.Invoke unless you want to change your code to make use of async/await.

Dirk
  • 10,668
  • 2
  • 35
  • 49
1

It's fine as long as the UI thread isn't overburdened by those invokes. It does introduce some latency to the communication, which usually isn't an issue, however, it can become more of a problem if you're doing a lot of Invokes, or if the UI thread is doing a lot of work (eg. rendering complex graphs or something like that). Invoke is a synchronous method - it will not return until the invoked command is actually processed, and returns its return value.

As long as you're not tied up by these issues, all is well. Profiling and performance testing is critical to allocate your resources correctly, guessing is usually a huge waste of time and resources.

If you don't need the resulting value (or at least not synchronously) and you're starting to get into performance trouble, have a look at BeginInvoke, which handles the invoking asynchronously. This means your networking thread doesn't have to wait for the UI thread to work. This is quite critical in high performance servers with thousands of connections. They simply can't afford to wait while the UI does its thing.

However, do note, that having a server socket running on a different thread is not a good solution for larger servers, and in fact, it's no longer the easiest solution either. .NET now has great support for asynchronous calls and callbacks, making implementations of asynchronous processing a breeze. In your typical Winforms application, it means that I/O blocking applications can work without having constantly running and polling threads. For example, waiting for a new connection can be as simple as:

var connection = await listener.AcceptTcpClientAsync();

That's it. Automagically, all the callbacks will be processed at the right time, without blocking the processing, all of your own code always running on the main UI thread. In other words, you can easily do this:

while (!aborted)
{
  var connection = await listener.AcceptTcpClientAsync();

  tbxLog.Text += "New connection!\r\n";
}

While this seems like an infinite loop blocking the UI thread indefinitely, the reality is that when the application gets to the await keyword, it will register an asynchronous callback and returns. Only when the asynchronous callback is actually invoked (by IOCP in this case) is the code resumed (on the UI thread), and tbxLog has the text appended, followed by waiting for another connection.

Luaan
  • 62,244
  • 7
  • 97
  • 116
0

I've never had problems doing it this way. No matter how you set it up, updating your controls has to be done on the thread they were created on. If you use a BackgroundWorker or some other async construct, somewhere an invoke is going to be called. I typically create a method on the form like:

delegate void TextSetter(string text);
internal void SetText(string text)
{
  //call on main thread if necessary
  if (InvokeRequired)
  {
    this.Invoke((TextSetter)SetText, text);
    return;
  }

  //set the text on your label or whatever
  this.StatusLabel.Text = text;
}

I've used that method in a number of applications and it's never been a problem, even updating many times per second.

As far as I'm aware, the only way to get around calling an invoke is to have your main thread constantly poll for updates, which is generally accepted as a really bad way to do things.

HypnoToad
  • 585
  • 1
  • 6
  • 18
0

A really obvious simplification is to abstract away the InvokeRequired/Invoke into an extension method for a Control.

public static class FormExt {
    public static void Execute(this Control c, Action a) {
        if (c.InvokeRequired) {
            c.Invoke(a);
        } else {
            a();
        }
    }
}

Now you just wrap up normal form updates into a lambda and execute them.

form1.Execute(() => form1.Text = "Hello world");
Mark H
  • 13,797
  • 4
  • 31
  • 45