Why lock(this) is bad

My last column has generated more questions again:

I am reading your article, "Write code for a multithreaded world". It is very interesting.
Could you please go more detail about
Locking the this object is always a bad idea?

The referenced article is here.

Well, the problem is one of how to manage the complexity involved with locks. When you say lock (this), the visibility of the object being locked matches the visibility of the class. So, if you have a public class, the object being locked is visible from everywhere.

Let's say you have a deadlock issue in your code. Well, if your locking objects are all private to a class you have done quite a bit to limit where you need to look. Chances are some method in your class acquires the lock, and then raises an event while holding the lock. The event handler may switch threads (through Control.Invoke) and that handler calls another method that tries to acquire the same lock.

It's still a tough problem, but there are a small number of ways to hurt yourself, and when you do, you've got a bounded set of code (that one class) where you need to look.

On the other hand, suppose a class is littered with lock(this) and you have a deadlock. Now, you need to search the entire codebase for lock constructs and examine each one to see if it's locking that object. Instead of one class, you could have hundreds of thousands of lines of code to search. It's a needle in a haystack, and the likelihood of finding the cause is the same.

So, prefer locking on a private member variable rather than locking this.

Published 09 September 2007 10:32 PM by wwagner
Filed under: ,

Comments

# Leo TOhill said on 10 September, 2007 09:44 AM

If I understand you correctly, lock(this) is only a problem if there is also code that does lock(that), where "that" is some non-private.  If all locks were on "this", the debugging problem you describe doesn't occur, right?

# wwagner said on 13 September, 2007 01:11 PM

You're right, but never locking this is easier to enforce than always locking this.

If all my locks are on private fields, then no other developer can break it. If all my locks are lock(this), any client code can break it.

# Luggage locks » Guard Lock said on 06 November, 2007 08:54 AM

Pingback from  Luggage locks » Guard Lock

# Luggage locks » Fetch-and-add said on 06 November, 2007 09:13 AM

Pingback from  Luggage locks » Fetch-and-add

# Luggage locks » Multiple granularity locking said on 15 November, 2007 11:38 AM

Pingback from  Luggage locks » Multiple granularity locking

# Luggage locks » Lock up period said on 16 November, 2007 10:53 PM

Pingback from  Luggage locks » Lock up period

Search

Go

Blog Group Links

Nascar style badges