Introduction
Locking is important for synchronizing access to data shared between threads. Ideally, you want to avoid the need for this but sometimes it is not possible or practical. So instead, we must lock - and unlock - correctly, and ideally without cluttering up the codebase.
C# provides us the lock(...) {}
statement. Trouble is, this has pitfalls: notably because it blocks the current thread until it actually acquires the lock. If another thread has locked an object and (for example) is stuck in an infinite loop, when you attempt to lock the same object, you will wait on it forever because lock
does not time out - and the application will be deadlocked.
In my opinion, it is better to use timeouts so your code will not wait forever if something goes wrong, so a better strategy is to use
Monitor.TryEnter
, and include a maximum timeout (you did declare a constant for it, instead of hardcoding - right?). Then, you add a
try
/
finally
so you will always call
Monitor.Exit
afterward.
All told, you will end up with a lot of surrounding code like this:
private readonly object syncObject = new object();
private const int TimeoutMaxWait = 50;
if (Monitor.TryEnter(syncObject, TimeoutMaxWait))
{
try
{
}
finally
{
Monitor.Exit(syncObject);
}
}
else
{
}
Now, there is nothing wrong with this, but if we have a class with a lot of methods that require locking, we will have a lot of this boilerplate code. Maybe we could do something about it, and reduce the above code to:
using (Lock())
{
}
Using the Code
Whether this works for you is heavily dependent on what you do when it fails to lock - I throw an exception in all cases. If you want to do something specific at each call site, your code will be more complex.
The main piece behind this solution is an IDisposable
object, Key
which holds a reference to the object we locked on (this allows re-use across multiple classes). If we put the Key
in a using
statement, it will unlock itself.
public class Key : IDisposable
{
private object padlock;
public Key(object locker)
{
padlock = locker;
}
public void Dispose()
{
Monitor.Exit(padlock);
}
}
One Way - Less Code
Then add a method to the class to acquire and return a Key
.
protected Key Lock()
{
if (System.Threading.Monitor.TryEnter(this.syncObject, LockTime))
return new Key(this.syncObject);
else
throw new System.TimeoutException("(class) failed to acquire the lock.");
}
With this method, just wrap your work with a single using
call.
using (Lock())
{ ... }
Again, I throw exceptions because it fits the component where I am using this code. The error message could easily be made into a parameter.
Another Way - DIY Failure
If you want to handle failure your way, create a Key
manually which still gives you the try
, finally
and Monitor.Exit
calls for free. Your locking code would look like:
if (Monitor.TryEnter(this.syncObject, LockTime))
{
using (new Key(this.syncObject))
{
}
}
else
{
}
That's a bit cleaner than the original, and most importantly - you can't forget to unlock it.
Note - What to Lock
You should always lock on a private readonly
object, ideally one which is kept for the purpose of synchronization. I have seen code examples like this:
lock (myList)
{
myList = new List();
myList.AddRange(something);
}
If the thread is pre-empted and someone now tries to lock on myList
, it's a different object - so the locking is ineffective.
Locking on this
or on typeof(...)
is a practice that is discouraged by MSDN because these are not necessarily safe to use for locks.
History
- 2014/4/11: Initial version