1

Just to put the questions upfront (please no comments about the bad architecture or how to revise - suppose this is what it is):

  1. How does the "lock" statement apply when using Invoke/BeginInvoke
  2. Could the following code result in a deadlock?

Suppose I have the following BindingList that I need to update on the GUI Thread:

var AllItems = new BindingList<Item>();

I want to make sure that all updates to it are synchronized. Suppose I have the following subroutine to do some calculations and then insert a new entry into the BindingList:

private void MyFunc() {
  lock(locker) {
    ... //do some calculations with AllItems

    AddToArray(new Item(pos.ItemNo));

    ... //update some other structures with the contents of AllItems
    }
}

And AddToArray looks like:

private void AddToArray (Item pitem)
{
   DoInGuiThread(() =>
     {
       lock (locker)
         {
           AllItems.Add(pitem);
         }
      });
 }

And DoInGuiThread looks like:

 private void DoInGuiThread(Action action) {
   if(InvokeRequired) {
       BeginInvoke(action);
    } else {
       action.Invoke();
    }
 }
Denis
  • 11,796
  • 16
  • 88
  • 150

2 Answers2

3

The lock is held till you leave the lock block, your current code does not cause a deadlock however it also does not work correctly either.

Here is the sequence of events:

  1. On a background thread you call MyFunc.
  2. A lock is taken for the background thread for the object locker
  3. The background thread will "do some calculations with AllItems"
  4. The background thread calls AddToArray from MyFunc passing in pitem
  5. The background thread calls DoInGuiThread from AddToArray
  6. The background thread calls BeginInvoke from DoInGuiThread, the thread does not block, I will use A to signify the background thread and B to signify the UI thread, both of these are happening at the same time.

  7. A) BeginInvoke returns from it's call because it is non blocking.
    B) The UI hits lock (locker) and blocks because the lock is held by the background thread.

  8. A) DoInGuiThread returns.
    B) The UI is still locked up, waiting for the background thread to release the lock.
  9. A) AddToArray returns.
    B) The UI is still locked up, waiting for the background thread to release the lock.
  10. A) The background thread will "update some other structures with the contents of AllItems" (note, pitem has not yet been added to AllItems)
    B) The UI is still locked up, waiting for the background thread to release the lock.
  11. A) The background thread releases the lock for the object locker
    B) The UI thread takes the lock for the object locker
  12. A) MyFunc returns.
    B) pitem is added to AllItems
  13. A) Whoever called MyFunc continues to run code
    B) The UI thread releases the lock for the object locker
  14. A) Whoever called MyFunc continues to run code
    B) The UI thread returns to the message pump to process new messages and no longer appears to be "locked up" by the user.

Do you see the issue? AddToArray returns but the object is not added to the array until the end of MyFunc so your code after AddToArray will not have the item in the array.

The "usual" way to solve this is you use Invoke instead of BeginInvoke however that causes a deadlock to happen. This is the sequence of events, Steps up to 6 are the same and will be skipped.

  1. The background thread calls Invoke from DoInGuiThread
  2. A) Invoke waits for B to return to the message pump.
    B) The UI hits lock (locker) and blocks because the lock is held by the background thread.
  3. A) Invoke waits for B to return to the message pump.
    B) The UI is still locked up, waiting for the background thread to release the lock.
  4. A) Invoke waits for B to return to the message pump.
    B) The UI is still locked up, waiting for the background thread to release the lock.
  5. A) Invoke waits for B to return to the message pump.
    B) The UI is still locked up, waiting for the background thread to release the lock.

(This repeats forever)

Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
1

There's two different ways this is likely to go down.

  1. You're doing all of this on the GUI thread
  2. You're starting this call chain on some other thread

Let's deal with the first first.

In this case there won't be a problem. You take the lock in MyFunc, you call AddToArray which calls DoInGuiThread passing in a delegate. DoInGuiThread will notice that invoking is not required and call the delegate. The delegate, executing on the same thread that now holds the lock, is allowed to enter the lock again, before calling AllItems.Add.

So no problem here.

Now, the second case, you start this call chain on some other thread.

MyFunc starts by taking the lock, call AddToArray which calls DoInGuiThread passing the delegate. Since DoInGuiThread now detects that it needs to invoke it calls BeginInvoke passing in the delegate.

This delegate is queued on the GUI thread by ways of a message. Here's where things again diverge. Let's say the GUI thread is currently busy, so it won't be able to process messages for a short while (which in this context means "enough to let the rest of this explanation unfold").

DoInGuiThread, having done its job, returns. The message is not yet processed. DoInGuiThread returned back to AddToArray which now returns back to MyFunc which releases the lock.

When the message is finally processed, nobody owns the lock, so the delegate being called is allowed to enter the lock.

Now, if the message ended up being processed before the other thread managed to return all the way out of the lock, the delegate now executing on the GUI thread would simply have to wait.

In other words, the GUI thread would block inside the delegate, waiting for the lock to be released so it could be entered by the code in the delegate.

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825