Is it possible to create a deadlock in C# if nothing but the lock keyword is used around primitive data access?

Question

I've written a lot of multi-threaded C# code, and I've never had a deadlock in any code I've released.

I use the following rules of thumb:

1. I tend to use nothing but the lock keyword (I also use other techniques such as reader/writer locks, but sparingly, and only if required for speed).
2. I use Interlocked.Increment if I am dealing with a long.
3. I tend to use the smallest granular unit of locking: I only tend to lock around primitive data structures such as long, dictionary or list.

I'm wondering if it's even possible to generate a deadlock if these rules are thumb are consistently followed, and if so, what the code would look like?

Update

I also use these rules of thumb:

4. Avoid adding a lock around anything that could pause indefinitely, especially I/O operations. If you absolutely have to do so, ensure that absolutely everything within the lock will time out after a set TimeSpan.
5. The objects I use for locking are always dedicated objects, e.g. object _lockDict = new object(); then lock(_lockDict) { // Access dictionary here }.

Update

Great answer from Jon Skeet. It also confirms why I never get deadlocks as I tend to instinctively avoid nested locks, and even if I do use them, I've always instinctively kept the entry order consistent.

And in response to my comment on tending to use nothing but the lock keyword, i.e. using Dictionary + lock instead of ConcurrentDictionary, Jon Skeet made this comment:

@Contango: That's exactly the approach I'd take too. I'd go for simple code with locking over "clever" lock-free code every time, until there's evidence that it's causing an issue.

Answer 1

Yes, it's easy to deadlock, without actually accessing any data:

private readonly object lock1 = new object();
private readonly object lock2 = new object();

public void Method1()
{
    lock(lock1)
    {
        Thread.Sleep(1000);
        lock(lock2)
        {
        }
    }
}

public void Method2()
{
    lock(lock2)
    {
        Thread.Sleep(1000);
        lock(lock1)
        {
        }
    }
}

Call both Method1 and Method2 at roughly the same time, and boom - deadlock. Each thread will be waiting for the "inner" lock, which the other thread has acquired as its "outer" lock.

If you make sure you always acquire locks in the same order (e.g. "never acquire lock2 unless you already own lock1) and release the locks in the reverse order (which is implicit if you're acquiring/releasing with lock) then you won't get that sort of deadlock.

You can still get a deadlock with async code, with just a single thread involved - but that involves Task as well:

public async Task FooAsync()
{
    BarAsync().Wait(); // Don't do this!
}

public async Task BarAsync()
{
    await Task.Delay(1000);
}

If you run that code from a WinForms thread, you'll deadlock in a single thread - FooAsync will be blocking on the task returned by BarAsync, and the continuation for BarAsync won't be able to run because it's waiting to get back onto the UI thread. Basically, you shouldn't issue blocking calls from the UI thread...

Answer 2

As long as you ever only lock on one thing it's impossible, if one thread tries to lock on multiple locks, then yes. The dining philosophers problem nicely illustrates a simple deadlock caused with simple data.

As the other answers have already shown;

void Thread1Method()
{
   lock (lock1)
   {
     // Do smth
     lock (lock2)
     { }
   }
}

void Thread2Method()
{
   lock (lock2)
   {
     // Do smth
     lock (lock2) 
     {  }
   }
}

Answer 3

Addendum to what Skeet wrote:

The problem normally isn't with "only" two locks... (clearly there could be even with only two locks, but we want to play in Hard mode :-) )...

Let's say that in your program there are 10 lockable resources... Let's call them a1...a10. You must be sure that you'll always lock those in the same order, even for subsets of them... If a method needs a3, a5 and a7, and another methods needs a4, a5, a7, you must be sure that both will try locking them in the "right" order. For simplicity sake in this case the order is clear: a1->a10.

Normally lock objects aren't numbered, and/or they aren't taken in a single method... For example:

void MethodA()
{
    lock (Lock1)
    {
        CommonMethod();
    }
}

void MethodB()
{
    lock (Lock3)
    {
        CommonMethod();
    }
}

void CommonMethod()
{
    lock (Lock2)
    {
    }
}

void MethodC()
{
    lock (Lock1)
    {
        lock (Lock2)
        {
            lock (Lock3)
            {
            }
        }
    }
}

Here, even with the Lock* numbered, it isn't immediately clear that the locks could be taken in the wrong order (MethodB+CommonMethod take Lock3+Lock2, while MethodC takes Lock1+Lock2+Lock3)... It isn't immediately clear and we are playing with three very big advantages: we are speaking of deadlock, so we are looking for them, the locks are numbered and the whole code is around 30 lines.