When would you ever use nested locking?

Question

I was reading in Albahari's excellent eBook on threading and came across the following scenario he mentions that "a thread can repeatedly lock the same object in a nested (reentrant) fashion"

lock (locker)
  lock (locker)
    lock (locker)
    {
       // Do something...
    }

as well as

static readonly object _locker = new object();

static void Main()
{
  lock (_locker)
  {
     AnotherMethod();
     // We still have the lock - because locks are reentrant.
  }
}

static void AnotherMethod()
{
  lock (_locker) { Console.WriteLine ("Another method"); }
}

From the explanation, any threads will block on the first (outermost) lock and that it is unlocked only after the outer lock has exited.

He states "nested locking is useful when one method calls another within a lock"

Why is this useful? When would you NEED to do this and what problem does it solve?

Answer 1

Lets say you have two public methods, A() and B(), which both need the same lock.

Furthermore, let's say that A() calls B()

Since the client can also call B() directly, you need to lock in both methods.
Therefore, when A() is called, B() will take the lock a second time.

Answer 2

It's not so much that it's useful to do so, as it's useful to be allowed to. Consider how you may often have public methods that call other public methods. If the public method called into locks, and the public method calling into it needs to lock on the wider scope of what it does, then being able to use recursive locks means you can do so.

There are some cases where you might feel like using two lock objects, but you're going to be using them together and hence if you make a mistake, there's a big risk of deadlock. If you can deal with the wider scope being given to the lock, then using the same object for both cases - and recursing in those cases where you'd be using both objects - will remove those particular deadlocks.

However...

This usefulness is debatable.

On the first case, I'll quote from Joe Duffy:

Recursion typically indicates an over-simplification in your synchronization design that often leads to less reliable code. Some designs use lock recursion as a way to avoid splitting functions into those that take locks and those that assume locks are already taken. This can admittedly lead to a reduction in code size and therefore a shorter time-to-write, but results in a more brittle design in the end. It is always a better idea to factor code into public entry-points that take non-recursive locks, and internal worker functions that assert a lock is held. Recursive lock calls are redundant work that contributes to raw performance overhead. But worse, depending on recursion can make it more difficult to understand the synchronization behavior of your program, in particular at what boundaries invariants are supposed to hold. Usually we’d like to say that the first line after a lock acquisition represents an invariant “safe point” for an object, but as soon as recursion is introduced this statement can no longer be made confidently. This in turn makes it more difficult to ensure correct and reliable behavior when dynamically composed.

(Joe has more to say on the topic elsewhere in his blog, and in his book on concurrent programming).

The second case is balanced by the cases where recursive lock entry just makes different types of deadlock happen, or push up the rate of contention so high that there might as well be deadlocks (This guy says he'd prefer it just to hit a deadlock the first time you recursed, I disagree - I'd much prefer it just to throw a big exception that brought my app down with a nice stack-trace).

One of the worse things, is it simplifies at the wrong time: When you're writing code it can be simpler to use lock recursion than to split things out more and think more deeply about just what should be locking when. However, when you're debugging code, the fact that leaving a lock does not mean leaving that lock complicates things. What a bad way around - it's when we think we know what we're doing that complicated code is a temptation to be enjoyed in your off-time so you don't indulge while on the clock, and when we realised we messed up that we most want things to be nice and simple.

You really don't want to mix them with condition variables.

Hey, POSIX-threads only has them because of a dare!

At least the lock keyword means we avoid the possibility of not having matching Monitor.Exit()s for every Monitor.Enter()s which makes some of the risks less likely. Up until the time you need to do something outside of that model.

With more recent locking classes, .NET does it's bit to help people avoid using lock-recursion, without blocking those who use older coding patterns. ReaderWriterLockSlim has a constructor overload that lets you use it recursion, but the default is LockRecursionPolicy.NoRecursion.

Often in dealing with issues of concurrency we have to make a decision between a more fraught technique that could potentially give us better concurrency but which requires much more care to be sure of correctness vs a simpler technique that could potentially give worse concurrency but where it is easier to be sure of the correctness. Using locks recursively gives us a technique where we will hold locks longer and have less good concurrency, and also be less sure of correctness and have harder debugging.