Bad Locking Patterns


Multithreading adds the overhead of making sure data is accessed in a thread safe way. Win32 API uses the CRITICAL_SECTION structure and functions. CLR provides mutual exclusive locks with Monitor and sync blocks. Basically, the Monitor class provides the same mechanism as CRITICAL_SECTION. How it works under the hood: every object in managed world has two overhead fields, one for the type object pointer and the second one for a thin lock (embedding locking information as a bit pattern) or an index in an array of sync blocks (in case the thin lock in not enough). The second field is used for thread synchronization.


The C# lock statement and the Visual Basic SyncLock statement are syntactic sugar. Given an object myObject, lock(myObject) is equivalent to:


Monitor.Enter(myObject);


Try {


       …..


}


Finally {


       Monitor.Exit(myObject);


}


Monitor.Enter can take any object; any caller can lock on any object. This requires extra care, because different threads locking on the same object's SyncBlock can cause a deadlock.


There are some great posts out there on the most “dangerous” locking techniques. However, I keep seeing lock(this) or lock(typeof(MyType)) or lock(“MyString”), so I figured it won’t hurt for me to talk about them too.


The MSDN documentation for lock statement tells us to avoid locking on a public type, or instances beyond your code's control. The common constructs lock (this), lock (typeof (MyType)), and lock ("myLock") violate this guideline:



  1. lock (this) is a problem if the instance can be accessed publicly.

  2. lock (typeof (MyType)) is a problem if MyType is publicly accessible.

  3. lock(“myLock”) is a problem since any other code in the process using the same string, will share the same lock.

Best practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.


Let’s look at these 3 un-recommended practices. Most problems arise from the fact that we share objects across app domains and locking on them can interfere with each other. Visit cbrumme's WebLog to read more about app domains concepts (and learn more about domain-neutrality, instance agility etc).


1.       lock(this). If the instance is available outside your code, anyone can lock on it – and this is a recipe for trouble. An object’s SyncBlock is not private; when you lock(this), you use the current object’s sync block; if an external class locks on the same object, deadlocks can occur. Here is an article that goes into more details.


2.       lock (typeof (MyType)). There could be many problems with this construct. First of all, types are agile – the same type object is used in all app domains. So if the same code is running in 2 app domains, it will lock on the same object – not what you had in mind. Types are public – so anyone anywhere can lock on a type. Plus, when using types you involve Reflection, which adds unnecessary complexity to your locking. Since you don’t own the type object, don’t rely on it for synchronization, because you don’t know who else can access it.


3.       lock(“myLock”). Strings are interned – there is one instance of any given string literal for the entire program. The exact same object for this literal is used in all running app domains, on all threads. Because strings are shared across all app domains, the danger is that someone else might be using the same string to lock on.


I would add another construct to the list:


4.       lock(myValueType). Instances of unboxed value types do not have the two overhead fields I talked previously (pointer to method table and sync block index). Because they don’t have a SyncBlock associated, in effect no synchronization happens. Monitor.Enter and Monitor.Exit require reference to an object on the heap. So the first step compilers do when seeing this code is to box the unboxed value type instance – so now we have an object with the 2 overhead fields. The problem is, for any Monitor.Enter, we get a new boxed object that the thread is using for synchronization - and therefore different objects are being locked and unlocked. If you want to synchronize access to an unboxed value type instance, you must first allocate a System.Object object and use it for synchronization.


As a general rule, it's bad to lock an object you didn't create and don't know who else might be accessing. Doing so invites deadlocks and hard to debug issues. As the msdn documentation suggests, the best practice is to only lock private objects.


If you get into deadlock or hang situations, you can use WinDbg and SOS to investigate them. The !syncbk function from SOS is the first thing to try out.


0:023> !syncblk                 


Index         SyncBlock MonitorHeld Recursion Owning Thread Info          SyncBlock Owner


   40 0000000000165120          793         1 00000000001a6640  12e8  23   00000000c00bbf88 System.Object


 3197 0000000004d0fe50            3         1 000000000012e720   dd4  11   00000000c00bc238 System.Object


-----------------------------


Total           5986


CCW             3


RCW             0


ComClassFactory 0


Free            2543


The sosex extension for WinDbg has a useful !dlk command to find out if there are any deadlocks.

If you want to read more about thread synchronization, I highly recommend Jeffrey Richter’s article Safe Thread Synchronization.

Skip to main content