2

I am an enthusiast developer rather than a professional one, and as such I don't have a highly technical understanding of why some issues are as they are. Recently, I've been working on applications where I've been trying to improve responsiveness through use of backgroundworkers to handle various database calls.

Once the backgroundworkers complete, I need to update the UI, and so I normally do something like the following:

    private delegate void safeSetTextDelegate(Control control, string text);
    public static void SafeSetText(this Control control, string text)
    {
        if(control.InvokeRequired)
        {
            safeSetTextDelegate _delegate = new safeSetTextDelegate(SafeSetText);
            control.Invoke(_delegate, new object[] { control, text});
        }
        else
        {
            control.Text = text;
        }
    }

Now, obviously this works. But as the number of extension methods I find that I need starts to grow (I have them for setting styles, adding child controls dynamically, adding rows to datagridviews etc), I was wondering why there isn't a set of standard methods available in the .Net WinForms language / model for these types of things, or even why the InvokeRequired checks aren't made automatically behind-the-scenes and a delegate applied as required (my guess is that this is a performance issue - why check the calling thread when in the majority of cases it simply isn't required). I have tried to research this, and haven't found any decent answers.

If I was to summarize this into a set of questions:

  1. Is there anything inherently bad / wrong in my approach (backgroundworkers updating controls via extension methods that check InvokeRequired)
  2. Is my thinking as to why the various methods in the .Net WinForms don't automatically do InvokeRequired checks correct (how much overhead is there in calling InvokeRequired)?
  3. Similarly, is there are reason why thread-safe methods for common UI updates don't exist in the .Net WinForms specification?
ainwood
  • 992
  • 7
  • 17
  • 3
    backgroundworkers progress event and completed event are run on the UI thread, so you don't need `Invoke` if they are called from those methods. – Poul Bak Mar 01 '22 at 01:02
  • i have similiar curiosity, why not invoke automatically in the framework? – Lei Yang Mar 01 '22 at 01:03
  • 1
    You shouldn't be using `Invoke` inside the `DoWork`. `BackgroundWorker` already has a perfectly good `ReportProgress` event in which you can update the UI **without** any `Invoke` . Thread marshalling has already been performed for you. –  Mar 01 '22 at 01:26
  • 2
    BTW, as a general rule you should use `BeginInvoke` rather than `Invoke` as the latter can lead to _thread deadlock_ –  Mar 01 '22 at 01:28
  • Btw the `BackgroundWorker` class is [technologically obsolete](https://stackoverflow.com/questions/12414601/async-await-vs-backgroundworker/64620920#64620920). And the `InvokeRequired` has the smell of a very old bottle of wine that has not aged well. – Theodor Zoulias Mar 01 '22 at 01:32
  • I probably over-simplified. I have base classes that populated from database queries, and collections to base classes. Backgroundworkers kick-off population of the collections, and this loading of the collection creates the base class instances. I use the BackGroundWorker.RunWorkerCompleted to tell me when the collection is loaded, and then I kick-off the UI updates from the Collection - which is creating controls dynamically, or populating DataGridViews. This is on a different thread, so I use Invoke. I guess I should try (learn) Async/Await. But how do I get around InvokeRequired? – ainwood Mar 01 '22 at 02:44
  • 1
    Using the `InvokeRequired` means that you don't know if you are on the UI thread or on a background thread. In a well designed application you always know where you are. If you are inside the `Task.Run` delegate, you are on the `ThreadPool`. If you are outside (before or after the `await Task.Run`), you are on the UI thread. You just need to be careful, and avoid any interaction with UI components when you are running on the `ThreadPool`. Trying to allow yourself the luxury of writing context-agnostic code (with the help of `InvokeRequired`), is a recipe for unmaintainability. – Theodor Zoulias Mar 01 '22 at 03:34
  • Actually, you don't always know where you are, even in a well-designed application. When you get into multi-threaded apps with Tasks and continuations, it's very rare that you're actually on the UI thread when you're doing work and need to invoke something on the main thread after a continuation. The OP is right; is ridiculous that the framework offloads the responsibility to callers check InvokeRequired, when the framework should know when Invoke needs to be called. It's just excess work and code for users of the framework. The important thing is to just not block the UI thread with locks. – Triynko May 09 '22 at 17:13

1 Answers1

1

waay waay too fiddly, say you have text you want to set in textBox1

   textBox1.Invoke(()=>{
         textBox1.Text = "New Value";
    }

if you are really concerned about overhead of doing the invoke when not needed (I wouldnt be) then do

    var act = {
         textBox1.Text = "New Value";
    }
    if(textBox1.InvokeRequired)
          textBox1.invoke(act);
    else 
       act();

I did this to see how much of an overhead

        for (int i = 0; i < 1000; i++)
            checkBox1.Invoke(act);

this took 0.0038 seconds.

versus

     for(int i = 0; i< 1000; i++){
            checkBox1.Checked = true;
        }

this took 0.0000050 seconds

So there is a non trivial overhead but not much. Since almost all winforms apps dont need that overhead then imposing it for all seems a bit much

Why is it like this, its not thread safety (as in race conditions etc) its to do with the UI plumbing deep down inside windows

Having written a lot of winforms code I can say, yes it is annoying but the fix is simple (now that I have shown you what I would do)

pm100
  • 48,078
  • 23
  • 82
  • 145
  • if you wouldn't care about the overhead, why microsoft guys do (so that the don't put this code into library)? – Lei Yang Mar 01 '22 at 01:09
  • @LeiYang there will be cases where it matters a lot if every UI update went through this. But OP is talking about background tasks that take a long time, an extra few milliseconds wont matter – pm100 Mar 01 '22 at 01:12
  • can you also answer OP's question 2? – Lei Yang Mar 01 '22 at 01:13
  • 1
    You should use `BeginInvoke` rather than `Invoke` as the latter can lead to _thread deadlock_ –  Mar 01 '22 at 01:24
  • I appreciate that it looks fiddly, but I am trying to make it more reusable through extension methods: `Textbox1.SafeSetText("NewValue")`. I appreciate you providing the the timing test results. – ainwood Mar 01 '22 at 02:35