Smarter than lock(), Cleaner than TryEnter

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:

// at class-level
private readonly object syncObject = new object();
private const int TimeoutMaxWait = 50; // milliseconds

// at each location you require a lock
if (Monitor.TryEnter(syncObject, TimeoutMaxWait))
{
    try
    {
        // useful work
    }
    finally
    {
        Monitor.Exit(syncObject);
    }
}
else
{
    // failed to get lock: throw exceptions, log messages, get angry etc.
}  

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())
{
    // useful work
} 

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()
    {
        // when this falls out of scope (after a using {...} ), release the lock
        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 exception, log message, etc.
        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))
    {
        // do useful work
    }
}
else
{
    // do what you want when it fails
} 

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) // do not do this
{
    myList = new List(); // do not do this either
    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