How Lock statement works in C#?

Question

public class Test 
{
    public Test instance = null;
    public static readonly object lockA = new object();
    public static readonly object lockB = new object();
    public Dictionary<string, string> data = new Dictionary<string, string>();

    public static Test Instance 
    {
        get 
        {
            if(instance == null)
            {
                lock(lockA)
                {
                    instance = new Test();
                }
            }
            
            return instance;
        }
    }
    
    
    public async Task Init()
    {
        // fetch and populate data to the "data" dictionary
    }
    
    
    public string Get(string key)
    {
        if(data.ContainsKey(key))
        {
            return data[key];
        }
        else
        {
            return key;
        }
    }
    
    
    public void Update(string key, string value)
    {
        lock(lockB)
        {
            // some other code lines...

            data.Add(key, value); 
        }
    }
}

Let's take a scenario that, multiple users gonna consume this at a same time. In this case, Thread A is updating the data Dictionary. It means Thread A is inside the body of lock in Update method. There for any other thread won't be able to execute the body of the lock of Update method while thread A completes.

Now thread B,C and D wants to read the data from Dictionary at the same time.

Test.Instance.Get("SomeKey");

So my confusion is, should Thread B,C, and D have to wait until Thread A release the lock in Update method since Thread A uses data dictionary inside the lock statement?

As my understand, lock statement only not allow other threads to access the body of the lock if its acquired by some other thread right?

Please explain this.

Answer 1

Let me address one of your comments first up:

In my case I need multiple threads concurrently read the data dictionary. Its okay in my case. No need to synchronized. But when it comes to update, it should be synchronized.

It can be done, but it's not trivial, and potentially leads to issues you won't expect. Here's one option for this:

class Test
{
    private readonly object dataLock = new();
    private volatile Dictionary<string, string> data = new();

    // Ignoring all the other parts for now.

    public string ShortGet(string key)
    {
        // Since we only reference 'data' once we don't need special handling
        return data.TryGetValue(key, out var value) ? value : key;
    }
    
    public string LongGet(string key)
    {
        // Since we reference 'data' twice, make a local copy to ensure it
        // doesn't change between references
        var localData = data;
        if (localData.ContainsKey(key))
            return localData[key];
        return key;
    }

    public void Update(string key, string value)
    {
        lock (dataLock)
        {
            // Don't duplicate if the value is the same
            if (data.TryGetValue(key, out var current) && current == value)
                return;
            // Don't insert if key==value
            if (key == value)
                return;
            
            // Dupluicate the existing dictionary, update it, then switch
            // for the original
            var newData = new Dictionary<string, string>(data);
            newData[key] = value;
            Interlocked.Exchange(ref data, newData);
        }
    }
}

Neither ShortGet() nor LongGet() use locks, so they won't block at all. The Update() method locks for the duration of the operation, so only one thread can perform an update at a time.

The downside here is that you have to create a full copy of data each time you want to update a value. I've added some simple (and badly done) checks to maybe reduce that occasionally.

---

Now, let's explain locking.

The lock (...) { } statement attempts to acquire a thread-specific lock on the object before executing the code in the block. If the current thread already has a lock it'll pass through to the code block, otherwise if the object is locked by another thread it will block until the other thread releases the lock. And of course if no thread has it locked, the thread acquires a lock on the object. At the end of the code block if the lock was acquired in this specific lock statement then it is released from the thread.

And that is, in very broad details, everything that a lock statement does. It doesn't restrict access to the object to other things, it doesn't stop other threads from doing nasty things to the object without your consent. It just acquires and releases locks.

Now let's look at your Instance property and supporting objects:

class Test
{
    public static Test instance = null;
    public static readonly object lockA = new object();
    public static Test Instance
    {
        get
        {
            if (instance == null)
            {
                lock (lockA)
                {
                    instance = new Test();
                }
            }
            return instance;
        }
    }
}

First, both of the static fields (instance and lockA) should be private. If something can write to the instance field at any time without having to honor the lock, or something can acquire a lock from outside of your code, then all bets are off.

Now, in the Instance getter you're accessing the instance field three time: two reads and a write. And only the write is protected. This leaves it open for race conditions where two or more threads are trying to read and write at the same time.

Let's trace what happens when two threads try to get the value of Instance at the exact same time. In an ugly table.

Thread A	Thread B	instance	Notes
if (instance == null)		null
	if (instance == null)	null
lock (lockA)		null
	lock (lockA)	null	B blocked until A releases
instance = new Test();		{Test#1}	Create first instance
	instance = new Test();	{Test#2}	Create second instance
return instance;		{Test#2}	returns second instance
	return instance;	{Test#2}	returns second instance

(That's assuming that the threads are acting interleaved at the line level... unlikely, but possible.)

So you've created two different instances, one of which got lost. Could we make it worse? Sure. Let's delay the execution of the second thread by a line:

Thread A	Thread B	instance	Notes
if (instance == null)		null
lock (lockA)		null
	if (instance == null)	null
instance = new Test();		{Test#1}	Create first instance
	lock (lockA)	{Test#1}	B not blocked
return instance;		{Test#1}	returns first instance
	instance = new Test();	{Test#2}	Create second instance
	return instance;	{Test#2}	returns second instance

And now you have two completely separate instances of the Test class floating around, one of which is orphaned.

What you should be doing is locking the whole code of the getter, and doing the operation as quickly as possible. Like this:

    private static readonly object lockA;
    private static Test instance;

    public static Test Instance
    {
        get
        {
            lock (lockA)
            {
                return instance ??= new();
            }
        }
    }

Now all of the interactions with the instance field are behind a lock, nothing can interfere with instance or acquire a bad lock on lockA, so all of the issues with race conditions between threads should be resolved.

(And since I'm in nitpick mode: lockA is stunningly opaque. Why not call it instanceLock instead?)

Answer 2

lock statement only not allow other threads to access the body of the lock if its acquired by some other thread right?

Yes. The lock is a block of code where only one thread can enter at a time. While a thread is inside this block, other threads have to wait until the owning thread exits the block. The code inside a lock block is sometimes called critical section.

The lock protects code, not variables or objects. It's your responsibility to ensure that non-thread-safe shared state (like the data dictionary in your example) is always accessed inside lock protected sections with the same locker object. If you forget to protect even a single interaction with the shared state, allowing more than one threads to interact with it concurrently, the behavior of your program becomes undefined. In your example the data is not protected inside the Get method, and also the instance is read without protection inside the Test method, so your program has undefined behavior.