Get Even More Visitors To Your Blog, Upgrade To A Business Listing >>

Don’t Make This Dumb Locking Mistake

What’s wrong with this code:

try
{
    if (!Monitor.TryEnter(this.lockObj))
    {
        return;
    }
    
    DoWork();
}
finally
{
    Monitor.Exit(this.lockObj);
}

This is a rookie mistake, but sadly I made it the other day when I was fixing a bug in haste. The problem is that the Monitor.Exit in the finally block will try to exit the lock, even if the lock was never taken. This will cause an exception. Use a different TryEnter overload instead:

bool lockTaken = false;
try
{
    Monitor.TryEnter(this.lockObj, ref lockTaken))
    if (!lockTaken)
    {
        return;
    }
    
    DoWork();
}
finally
{
    if (lockTaken)
    {
        Monitor.Exit(this.lockObj);
    }
}

You might be tempted to use the same TryEnter overload we used before and rely on the return value to set lockTaken. That might be fine in this case, but it’s better to use the version shown here. It guarantees that lockTaken is always set, even in cases where an exception is thrown, in which case it will be set to false.


Check out my latest book, the essential, in-depth guide to performance for all .NET developers:

Writing High-Performance.NET Code by Ben Watson. Available now in print and as an eBook at:

  • Amazon
  • Barnes and Noble
  • and more, see book site


This post first appeared on Philosophical Geek | Code And Musings By Ben Watso, please read the originial post: here

Share the post

Don’t Make This Dumb Locking Mistake

×

Subscribe to Philosophical Geek | Code And Musings By Ben Watso

Get updates delivered right to your inbox!

Thank you for your subscription

×