Why Concurrency Is Hard (Or: TimedLock Can Get You in Trouble)
I’ve just noticed a post by Guy Kolbis discussing a possible solution for deadlocks – ensuring that all locks are taken with a timeout. To do so, Guy cites the TimedLock struct, originally introduced by Ian Griffiths.
The general idea is that instead of using a standard lock{…} block, you wrap your critical section in the following statement:
using (TimedLock.Lock(...))
{
}
Ian even introduces a refinement of this idea by including a sentinel reference type in the struct, in Debug builds, so that if the lock isn’t explicitly freed, the sentinel’s finalizer will be called and raise an assertion – so that the developer is immediately notified that there was a lock she forgot to release.
Unfortunately, there is a subtle yet fatal flaw with this approach. Consider the proposed constructor of TimedLock:
public static TimedLock Lock (object o, TimeSpan timeout)
{
TimedLock tl = new TimedLock (o);
if (!Monitor.TryEnter (o, timeout))
{
#if DEBUG
System.GC.SuppressFinalize(tl.leakDetector);
#endif
throw new LockTimeoutException ();
}
return tl;
Can you spot the problem?
Here’s a hint – what happens if an exception occurs after the lock was successfully acquired, but before the method returns?
Let’s step back for a second – how can an exception possibly occur between these two lines? After all, if the TryEnter method returned successfully, there are no other operations in this method that could possibly introduce an exception. Or maybe there are?
Of course there are! If your thread is asynchronously aborted, e.g. using the Thread.Abort method, or as a result of an asynchronous exception such as OutOfMemoryException, then the exception can be potentially introduced between any two instructions in your code, including the most innocuous ones.
And what happens then? Well, because the exception occurs before the method has returned, the using statement does not actually dispose the lock because the reference to the lock is never assigned in the caller’s code. If you’re running in Debug, you would probably catch it thanks to the sentinel reference type – but if this happens to you in a production scenario, you have just leaked an acquired lock, which is never going to be released.
You can easily emulate this by putting a trace statement in the Dispose method, introducing a short sleep into the static Lock method (between the two problematic lines we discussed above), and then inducing an asynchronous exception from a different thread while the Lock method is executing – e.g. by aborting that thread.
So essentially, while the TimedLock apparently solves the problem of deadlocks, it does so by reintroducing potential deadlocks. Not much gained here.
Nitpicker’s Corner
You, Dear Reader, might have a completely legitimate question. Namely: Why is the lock keyword immune from this problem?
As you probably know, a lock statement is compiled to a call to Monitor.Enter, and then a try…finally block. In the finally block, Monitor.Exit is called.
The JIT code generation for both x86 and x64 ensures that a thread abort cannot occur between a Monitor.Enter call and a try block that immediately follows it. This always worked that way for x86/Release (because there were no instructions between the call and the subsequent instruction within the try block), and was actually fixed in VS2008 for x64/Release. (What we have in our case is a try block that is quite a few instructions, including a function call return, away from the Monitor.Enter call.)