Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Deadlock when worker thread tries to invoke something on main thread

I am using an external component which periodically shoots events from a worker thread. In my event handler I use a Dispatcher to invoke some method on the main thread. This works nicely...

private void HandleXYZ(object sender, EventArgs e)
{
    ...
    if(OnTrigger != null)
        dispatcher.Invoke(OnTrigger, new TimeSpan(0, 0, 1), e);
}

However, when the program shuts down and the external component Dispose()s, the program sometimes hangs (and can only be seen and killed in the task manager).

When I look at what is happening it looks like "the component" is waiting for the event to return on the main thread (it stays in the Dispose() method), while the worker thread waits for the dispatcher to invoke the mentioned call to the main thread (it hangs in the dispatcher.Invoke-line).

For now I solved the shutdown problem by adding a timeout to the Invoke, which seems to work but feels wrong. Is there a cleaner way to do something like this? Can I force the main thread to take some time for jobs from other threads before shutting down?

I have tried to "disconnect" the event before shutting down, but that does not help, because the dispatcher is(could be) already waiting, when the program start to shut down...

PS: external component means here that I do not have access to the source code...

like image 406
FrankB Avatar asked Jul 19 '12 09:07

FrankB


2 Answers

Yes, this is a common source of deadlock. It hangs because the dispatcher exited the dispatcher loop it won't respond to Invoke requests anymore. A quick cure is to use BeginInvoke instead, it doesn't wait for the invoke target to finish executing. Another quickie is to set the worker thread's IsBackground property to True so the CLR will kill it.

These are quick fixes and they may well work for you. Certainly on your dev machine, but if you have a nagging feeling that it may still go wrong then you're right, not observing a deadlock or threading race does not prove they are not present. There are two "good" ways to do it completely safely:

  • don't allow the main thread to exit until you are sure that the worker thread terminated and can no longer raise events. This answer shows the pattern.

  • terminate the program forcefully with Environment.Exit(). This is very crude but very effective, a sledgehammer you'll only reach for when you have a heavily threaded program where the UI thread is only second citizen. Odd as this may sound as a suitable approach, the new C++ language standard has elevated it to a supported way to terminate a program. You can read more about it in this answer. Do note how it allows for cleanup functions to be registered, you'll have to do something similar with, say, the AppDomain.ProcessExit event. Focus on the first bullet before you do this.

like image 178
Hans Passant Avatar answered Nov 20 '22 13:11

Hans Passant


As for the event subscriptions, it is indeed a good idea to clean them up when you know that a particluar object is not needed anymore. Otherwise you would risk creating memory leaks. You might also want to have a look at the weak event pattern (MSDN).

Regarding the deadlock itself, without knowing your code, we can only guess.

I do not see the HandleXYZ() as a culprit, I would rather check your IDisposable() implemntaion. Have a look at the MSDN documentation and compare it to your implementation.

I suppose that somewhere in there in your implementation some method calls are made that depend on the timing of the GarbageCollector, which is indeterministic: Sometimes it may work out in your case, sometime it may not.

like image 1
Jens H Avatar answered Nov 20 '22 12:11

Jens H