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.
testVar
. – BendEgBest practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.
? – Vsevolod Goloviznin