Attempting to acquire a lock on an object when the calling method already has a lock on the same object

Question

I have some code that I have been going over to learn the system and I ran across some code that to me is a code smell and I wouldn't think it would work at all, but it does.

We have two objects, object A and object B. Object A contains a lock object:

private object lockObj = new object();

Object B will grab a lock on object A.lockObj and while B has the lock it calls

A.SomeMethod();

A.SomeMethod() acquires a lock on

this.lockObj

And to show it in code:

ThreadTestOne:

 public class ThreadTestOne
{
    public object lockObject = new object();

    private List<string> lst;

    private ThreadTestTwo two;

    public List<string> Lst
    {
        get
        {
            return this.lst;
        }
        set
        {
            this.lst = value;
        }
    }

    public void Run()
    {
        lst = new List<string>();
        two = new ThreadTestTwo();
        two.Run(this);
    }

    public void End()
    {
        Console.WriteLine("ThreadTestOne.End");
        two.End();
    }

    public void LockMe()
    {
        Console.WriteLine("ThreadTestOne.LockMe");
        lock (this.lockObject)
            lst.Add("something");
            Thread.Sleep(500);
    }
}

ThreadTestTwo:

public class ThreadTestTwo
{
    private ThreadTestOne one;
    private Thread myThread;
    private bool ending = false;

    public void Run(ThreadTestOne a)
    {
        one = a;
        myThread = new Thread(new ThreadStart(Consume));
        Console.WriteLine("ThreadTestTwo Starting thread");
        myThread.Start();
    }

    public void End()
    {
        Console.WriteLine("ThreadTestTwo.End");
        ending = true;
        myThread.Join();
    }

    public void Consume()
    {
        while (!ending)
        {
            Console.WriteLine("ThreadTestTwo one.lockObject");
            lock (one.lockObject)
            {
                Console.WriteLine("two.LockMe");
                one.LockMe();
                one.Lst.Add("two");
                Thread.Sleep(500);
            }
        }
    }
}

When I look over the above code, I think it should break as one.LockMe() should never be able to acquire a lock on lockObj because it ThreadTestTwo already has the lock.

I thought this would result in a deadlock. However, when I run the above example code, it works. Also, the code I was reviewing also works and is currently in production.

The fact that this doesn't result in an exception being thrown is confusing to me. Am I incorrect in assuming this should be an error?

In the code that I was testing originally only reading data after trying to acquire the lock twice so I had thought that the compiler was removing the lock.

However, I looked in the MSIL and saw that the lock is still there.

My next thought was the framework just wasn't acquiring the lock because we are just reading data.

I add a write operation within the lock and it still worked. However, it is possible that I don't fully understand how locking work.

Even though this works, I feel that it is wrong and I am not fully convinced that this will not cause issues in production.

I did find this question:

use the same lock object at two different code block?

Which is similar but I believe my issue is slightly different, I'm asking about locking an object when the calling method has already has a lock on that same object.

Obviously the code I have a question about works and I would like to know how?

Am I incorrect in assuming this is wrong?

There are a couple of issues that I am aware of in the above code.

1. public field - I know this is wrong, but that is how it is in the code.
2. Circular reference - I'm aware of the circular reference and know why it is bad.

Thank you for any insight you can provide.

Answer 1

You seem to be under the impression that a class owns a lock (aka monitor). That's not the case - a thread owns a monitor.

Monitors in .NET are re-entrant - if a thread already has the monitor, it can acquire it again. That will increase the "lock count" for it - when the thread releases the monitor the first time, it will just decrease the lock count, but as the count will still be positive, no other thread will be able to acquire the monitor until the original thread has released it again.

From Monitor.Enter (the method that the lock keyword sort-of calls - it actually calls TryEnter, but...):

It is legal for the same thread to invoke Enter more than once without it blocking; however, an equal number of Exit calls must be invoked before other threads waiting on the object will unblock.