Deadlock with one locked object?

Question

I have a multithreaded issue in C #. I am using an event to update a shortcut in a form from another thread, for which I need to use the Invoke () command, of course. This part also works great. However, the user can close the form, and here the program may crash if the event is sent at the wrong time.

So, I thought that just overriding the Dispose () method of the form, set the boolean true to the locked code, and also check that boolean and raise the event in the locked code.

However, every time I close the form, the program completely freezes.

Here are the pieces of code mentioned:

private object dispose_lock = new object(); private bool _disposed = false; private void update(object sender, EventArgs e) { if (InvokeRequired) { EventHandler handler = new EventHandler(update); lock (dispose_lock) { if (_disposed) return; Invoke(handler); // this is where it crashes without using the lock } return; } label.Text = "blah"; } protected override void Dispose(bool disposing) { eventfullObject.OnUpdate -= update; lock (dispose_lock) // this is where it seems to freeze { _disposed = true; // this is never called } base.Dispose(disposing); } 

I hope someone here knows what is wrong with this code. Thank you in advance!

Answer 1

I really would just be here. Instead of injecting tricky thread-safe code, I just catch the exception and do nothing if it doesn't work.

Assuming this is an ObjectDisposedException :

 try { this.Invoke(Invoke(handler)); } catch (ObjectDisposedException) { // Won't do anything here as // the object is not in the good state (diposed when closed) // so we can't invoke. } 

It is simpler and quite simple. If the comment indicates why you will catch the exception, I think that everything is in order.

Answer 2

What you do not take into account is that the delegate passed to Invoke is called asynchronously in the user interface thread. The Invoke call sends a message to the forms message queue and picks up after a while.

What's happening:

 UI Thread Background Thread Call update() take lock Call Invoke() Call update() release lock Call Dispose() take lock release lock 

Instead of this:

 UI Thread Background Thread Call update() take lock Call Invoke() block until UI Thread processes the message Process messages ... Dispose() wait for lock ****** Deadlock! ***** ... Call update() release lock 

Because of this, the background thread may hold the lock while the user interface thread tries to start Dispose

The solution is much simpler than you tried. Because Invoke is sent asynchronously, there is no need to block.

 private bool _disposed = false; private void update(object sender, EventArgs e) { if (InvokeRequired) { EventHandler handler = new EventHandler(update); Invoke(handler); return; } if (_disposed) return; label.Text = "blah"; } protected override void Dispose(bool disposing) { eventfullObject.OnUpdate -= update; _disposed = true; // this is never called base.Dispose(disposing); } 

The _disposed flag is only read or written in the user interface thread, so there is no need to block it. Now you call the stack like:

 UI Thread Background Thread Call update() take lock Call Invoke() block until UI Thread processes the message Process messages ... Dispose() _disposed = true; ... Call update() _disposed is true so do nothing 

Answer 3

One of the dangers of using Control.Invoke is that it can be removed in the user interface thread at the wrong time, as you suggested. The most common way this happens is when you have the following order of events

• Reference topic: Call queues with Invoke
• Foreground Thread: Dispose the control on which the background is called Invoke
• Foreground Thread: cancels a call on a remote control

In this case, Invoke will fail and throw an exception in the background thread. This was probably the reason that your application crashed in the first place.

With the new code, although this causes a deadlock. The code will lock in step # 1. Then, the removal occurs in the user interface in step 2 and expects a lock that will not be released until step # 3 is completed.

The easiest way to deal with this problem is to accept that Invoke is an operation that might not work, so try / catch is required

 private void update(object sender, EventArgs e) { if (InvokeRequired) { EventHandler handler = new EventHandler(update); try { Invoke(handler); } catch (Exception) { // Control disposed while invoking. Nothing to do } return; } label.Text = "blah"; } 

Answer 4

Why don't you just use BeginInvoke and not Invoke - this will not block the background thread. It doesn't seem that there is any specific reason why the background thread should wait for the UI update from what you showed.

Answer 5

Another deadlock scenario occurs when Dispatcher.Invoke (in the WPF application) or Control.Invoke (in the Windows Forms application) is called and is in possession of the lock. If another method works in the user interface that expects the same lock, a deadlock will occur there. This can often be fixed by simply calling the BeginInvoke call. In addition, you can release your lock before calling the Call, although this will not work if your caller has obtained a lock. We explain Invoke and BeginInvoke in Rich Client Applications and Thread Affinity.

source: http://www.albahari.com/threading/part2.aspx

Answer 6

IMO Dispose too late ...

I would recommend putting some code in FormClosing that FormClosing called before Dispose AFAIK happens.

In this case, I usually use a different (atomic) template for your verification - for example, through the Interlocked class.

 private long _runnable = 1; private void update(object sender, EventArgs e) { if (InvokeRequired) { EventHandler handler = new EventHandler(update); if (Interlocked.Read (ref _runnable) == 1) Invoke(handler); return; } label.Text = "blah"; } 

In FormClosing you simply call Interlocked.Increment (ref _runnable) .

Answer 7

Just don’t enter any of the other answers, culprit, is there any other code that terminates the thread that is not sent? I think you could use simple themes, not BackgroundWorker, and maybe forgot to set Thread.isBackround to true