CODEWITHSUNDEEP
c#.netmultithreadinglocking
BendEg
BendEg

Reputation: 21098

Is locking instance or member based

I have a question about locking in c#. Does c# lock an instance of an object, or the member.

If i have the following code:

lock(testVar)
{
    testVar = testVar.Where(Item => Item.Value == 1).ToList();
    //... do some more stuff
}

Does c# keep the lock, even i set testVar to a new value?

Upvotes: 4

Views: 699

Answers (3)

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149558

All C# objects inherit from System.Object, which itself always contains 4 bytes dedicated to be used when you use the syntactic sugar for lock. That's called a SyncBlock object.

When you create a new object using new, in your case, ToList which generated a new reference to a List<T>, you're actually overriding the old reference, which invalidates your lock. That means that now multiple threads could possibly be inside your lock. The compiler will transform your code into a try-finally block with an extra local variable, to avoid you from shooting your leg.

That is why the best practice is to define a dedicated private readonly variable which will act as a sync root object, instead of using a class member. That way, your intentions are clear to anyone reading your code.

Edit:

There is a nice article on MSDN which describes the objects structure in memory:

SyncTableEntry also stores a pointer to SyncBlock that contains useful information, but is rarely needed by all instances of an object. This information includes the object's lock, its hash code, any thunking data, and its AppDomain index. For most object instances, there will be no storage allocated for the actual SyncBlock and the syncblk number will be zero. This will change when the execution thread hits statements like lock(obj) or obj.GetHashCode.

Object in memory representation

Upvotes: 7

NeddySpaghetti
NeddySpaghetti

Reputation: 13495

The lock expression translates to a try/finally expression using Monitor.Enter/Monitor.Exit. Doing a simple test with some code similar to yours (with VS2015 Preview) you can see what the compiler translates the code to.

The code

var testVar = new List<int>();
lock (testVar)
{
    testVar = new List<int>();
    testVar.Add(1);
}

Is actually translated to this:

List<int> list2;
List<int> list = new List<int>();
bool lockTaken = false;
try
{
    list2 = list;
    Monitor.Enter(list2, ref lockTaken);
    list = new List<int> { 1 };
}
finally
{
    if (lockTaken)
    {
        Monitor.Exit(list2);
    }
}

So you can see that the compiler has completely removed your variable testVar and replaced it with 2 variables, namely list and list2. Then the following happens:

  1. list2 is initialized to list and now both references point to the same instance of List<int>.
  2. The call Monitor.Enter(list2, ref lockTaken) associates the synchronization block in the List<int> object with the current thread.
  3. The list variable is assigned to a new instance of List<int>, but list2 still points to the original instance that we locked against.
  4. The lock is release using list2

So even though you think that you are changing the lock variable, you are actually not. Doing that however makes your code hard to read and confusing so you should use a dedicated lock variable as suggested by the other posts.

Upvotes: 1

Marc Gravell
Marc Gravell

Reputation: 1063358

It locks on the object that the expression (testVar) resolves to. This means that your code does have a thread race, because once the list is reassigned, other concurrent threads could be locking on the new instance.

A good rule of thumb: only ever lock on a readonly field. testVar clearly isn't... but it could be, especially if you use RemoveAll to change the existing list instead of creating a new one. This of course depends on all access to the list happening inside the lock.

Frankly, though, most code doesn't need to be thread-safe. If code does need to be thread safe, the supported use scenarios must be clearly understood by the implementer.

Upvotes: 4

Related Questions