views:

72

answers:

3

I am working with a fairly complex GUI and am trying to pass a lot of data from the GUI to a backgroudWorker. The problem I am running into is accessing some of the GUI values from the background worker. For example, if I try to get ComboBox.Text I get a InvalidOperationException due to cross-threading. However, if I say do TextBox.Text, everything seems to work fine. Granted I am fairly new to C#, so I'm a little unclear on why some of these are OK and others fail.

I have come up with several ways to fix my issues but am seeking the best practice from someone who is experienced in c#.

Here are a couple ways i can think of fixing this

  1. create class/struct of all the values you want to pass to the background worker and pass this when you call RunworkAsync. I did not find this very attractive as i was having to build a class/struct for every page on my GUI to pass to the backgroundWorker

  2. Create a bunch of different background workers that had specific task. I still had some issues with passing data but the amount of data I had to pass was cut down quite a bit. However, the number of DoWork/ProgressChanged/RunworkerCompleted went up significantly which was less than ideal.

  3. (this lead me to what I'm currently doing)

create a delegate and method to capture the information

private delegate string ReadComboDelegate(ComboBox c);

private string ReadComboBox(ComboBox c)
{
    if(c.InvokeRequired)
    {
        ReadComboDelegate del = new ReadComboDelegate(this.ReadComboBox);
        return (string) c.Invoke(del,c);
    }
    else
    {
        return c.Text
    }
}

then within DoWork, do somthing like string txt = this.ReadComboBox(this.comboBox1);

When you have a simple GUI and you don't have to pass a lot of data this is pretty simple problem. However, the more items and complex the GUI gets the bigger this problem becomes. If anyone has any info that would make this easier, I would appreciate it.

Thanks

+2  A: 

The Cross Threading issue you are running into is due to the requirement that only the UI thread is allowed to "touch" UI controls.

I think that the most agreed upon method of passing data to a background worker is your solution #1 - create a simple structure that contains all of the data needed to perform the processing.

This is much simpler than creating ReadXXX methods for every control in the UI, and it defines what the background process needs to perform its task...

davisoa
+1  A: 

It is rather by accident that TextBox doesn't cause this exception. Its Text property is cached in a string. That's not the case for ComboBox.Text, and the vast majority of other control properties, it asks the native Windows control and at that point Windows Forms discovers that you are trying to use a control from a thread other than the UI thread. No can do.

You definitely need to think of a way to restructure this code. It is not only illegal, it is incredibly expensive and fundamentally thread unsafe since the UI could be updated while your worker is running. Collect the info from the controls you need into a little helper class, pass that as an argument to the RunWorkerAsync(object) overload. And get it back in DoWork from e.Argument.

Hans Passant
A: 

I would definitely avoid #3. Despite the fervor over using Control.Invoke to coordinate worker and UI threads it is often overused and is usually a suboptimal strategy at best. I much prefer #1 and #2 over #3. Here are the reasons why I tend to avoid #3.

  • It tightly couples the UI and worker threads.
  • The worker thread gets to dictate how much work the UI thread performs.
  • The worker thread has to wait for the UI thread to respond before proceeding.
  • It is an expensive operation.

I know it may require some additional upfront effort on your part to get #1 or #2 going, but the end result will be better in the long run.

As a corollary to my answer the Control.Invoke method tends to be overused when data needs to follow the opposite direction as well (from worker thread to UI thread as in the case of sending progress information to the UI). Sadly this is the method that BackgroundWorker uses internally with its ReportProgress method. It is usually better to have the UI thread poll a shared data structure for this information for some of the same reasons as above plus:

  • The UI thread gets to dictate when and how often the update should take place.
  • It puts the responsibility of updating the UI thread on the UI thread where it should belong anyway.
  • 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.

However, with that said I am not suggesting that you abandon BackgroundWorker entirely. Just keep some of these points in mind.

Brian Gideon