Using Control.Invoke() in place of lock(Control)

Question

I am working on a multi-threaded application which needs to update a Winforms control as it progresses (A DataGridView). In order to prevent multiple access to the same shared resource, I started out with the following lock structure:

if (DGV.Columns.Count >= DesiredColumnCount) return;
lock(DGV)
{
    while (DGV.Columns.Count < DesiredColumnCount)
    {
        DGV.Columns.Add(newColumn);
    }
}

I then realized that, since the DGV was created on the UI thread, it needs to be .Invoke()'d. This changes the code to:

if (DGV.Columns.Count >= DesiredColumnCount) return;
lock(DGV)
{
    DGV.Invoke(new MethodInvoker(() =>
        {
            while (DGV.Columns.Count < DesiredColumnCount)
            {
                DGV.Columns.Add(newColumn);
            }
        }
}

My question is: Isn't this redundant? the lock will block the worker thread until it has exclusive access to DGV, and the Invoke() will block the worker thread until the UI thread can pick up the invoke request and execute the code. Can't I just get by using only the Invoke()?

(That is the main question. Of course if there are any other multithreading sins in the above code, please comment)

You should never block the UI thread. That goes for lock as well as Control.Invoke. You should get rid of lock and change Invoke to BeginInvoke. — Peter Ritchie, May 15 '12 at 14:40
@PeterRitchie I don't remember exactly what started the thread. It was probably either `new Thread().Start()` or `System.Threading.Timer.Elapsed`. This code would not have blocked the UI thread, it would have blocked the worker. BeginInvoke cannot be blindly substituted for Invoke since it does not block the caller: I'd need to queue the invokes to make sure they didn't trip over each other from being executed in the wrong order. That might increase performance by reducing the time spent blocking worker threads, but this's an old project by now. — Nathan, May 21 '12 at 19:22

score 3 · Accepted Answer · edited May 15 '12 at 14:36

3

It is a bit redundant, the Invoke call will "dispatch" the operpation to the UI thread. Each subsequent Invoke call will be dispatched in a serial fashion, so there isn't really any need to do any locking.

You may consider using BeginInvoke instead of Invoke order to prevent the worker thread from blocking, but again, that will be done "serially" so there is no need to worry about locking.

edited May 15 '12 at 14:36

svick

236,525
50
385
514

answered Sep 28 '11 at 17:29

CodingGorilla

19,612
4
45
65

1

Thanks for the post, but I am confused. Mr. Skeet says in (http://stackoverflow.com/questions/229554/whats-the-difference-between-invoke-and-begininvoke) that Control.Invoke executes on the UI thread and waits for completion. – Nathan Sep 28 '11 at 17:34
Yes, he's talking about BeginInvoke(). Doesn't matter, you don't need a lock either way. All the code runs on the same thread so there is no need to protect the data. – Hans Passant Sep 28 '11 at 17:39
3

Not only is that lock redundant, it's a deadlock waiting to happen. If some code that is called during that call to `Invoke` tries to obtain the lock, you're stuck because your thread is holding the lock, waiting for the `Invoke` to complete on the UI thread, and the UI thread is waiting to acquire the lock. – Jim Mischel Sep 28 '11 at 17:47
@Nathan Sorry that was a poorly worded explanation, I reworded my answer. – CodingGorilla Sep 28 '11 at 17:48

Using Control.Invoke() in place of lock(Control)

1 Answers1

Linked