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

 •  0 comments  •  flag
Share on Twitter
Published on March 10, 2015 10:43
No comments have been added yet.


Ben Watson's Blog

Ben  Watson
Ben Watson isn't a Goodreads Author (yet), but they do have a blog, so here are some recent posts imported from their feed.
Follow Ben  Watson's blog with rss.