5

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...

FrankB
  • 349
  • 4
  • 10

2 Answers2

6

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.

Community
  • 1
  • 1
Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Good answer! I'd say the BeginInvoke-way feels better than the timeout. – FrankB Jul 19 '12 at 10:38
  • Does this mean: if the program was not shutting down, and the main thread would wait for the event... Would the dispatcher get the Invoke through to the main thread, although the main thread is "actively" waiting? – FrankB Jul 19 '12 at 10:54
  • Not sure how this is related to the original question. But any kind of "waiting" in the main thread is likely to cause deadlock too. An Invoke() call can only be dispatched if the main thread is idle and executing the dispatcher loop. – Hans Passant Jul 19 '12 at 11:04
  • Thanks! (The question is primarily related to your answer... But your answer tells me that the shutdown does not necessarily play a role in that deadlock - which is good to know) – FrankB Jul 19 '12 at 11:20
  • 1
    I have been using the sledgehammer for 30 years. Not allowing the main thread to exit until you are sure that the worker thread terminated is often difficult, impractical or actually virtually impossible. 'new C++ language standard has elevated it to a supported way to terminate a program' - that's because it's the only way that is sure to work - only the OS has the tools to terminate a thread in any state on any core. Unless I absolutely have to, I don't use synchronous comms like Invoke() and don't wait for anything on app close that might prevent the ExitProcess() being called. – Martin James Jul 19 '12 at 12:21
1

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.

Jens H
  • 4,590
  • 2
  • 25
  • 35
  • Thanks... but unfortunately the code of the Dispose() is not available to me (external component) – FrankB Jul 19 '12 at 10:07
  • 1
    The question is, @FrankB, do YOU do any cleanup? As a general rule, ALWAYS clean up any class that implements `IDispoeable()`. If that third-party component is waiting for something, it may well be the case, that your own code did not properly release some resources. – Jens H Jul 19 '12 at 10:13
  • That other component waits for the event to return, while my event handler waits for the main thread (which is blocked by the waiting component...). Cleaning up is only the solution if I can force the Dispatcher to stop waiting for the main thread. Can I do that other than setting a timeout? – FrankB Jul 19 '12 at 10:20