摘要:［待细读］Lock(this) is bad
http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java
－－－－－－－－－－－－－－－－－－－－－－－－
http://www.srtsolutions.com/why-lockthis-is-bad
lock(this) is bad
By: Bill WagnerMy 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.
－－－－－－－－－－－－－－－－－－－－－－
http://stackoverflow.com/questions/251391/why-is-lockthis-bad
It is bad form to use this in lock statements because it is generally out of your control who else might be locking on that object.
In order to properly plan parallel operations, special care should be taken to consider possible deadlock situations, and having an unknown number of lock entry points hinders this. For example, any one with a reference to the object can lock on it without the object designer/creator knowing about it. This increases the complexity of multi-threaded solutions and might affect their correctness.
A private field is usually a better option as the compiler will enforce access restrictions to it, and it will encapsulate the locking mechanism. Using this violates encapsulation by exposing part of your locking implementation to the public. It is also not clear that you will be acquiring a lock on this unless it has been documented. Even then, relying on documentation to prevent a problem is sub-optimal.
Finally, there is the common misconception that lock(this) actually modifies the object passed as a parameter, and in some way makes it read-only or inaccessible. This is false. The object passed as a parameter to lock merely serves as a key. If a lock is already being held on that key, the lock cannot be made; otherwise, the lock is allowed.
This is why it's bad to use strings as the keys in lock statements, since they are immutable and are shared/accessible across parts of the application. You should use a private variable instead, anObject instance will do nicely.
Run the following C# code as an example.
public class Person
{
public int Age { get; set; }
public string Name { get; set; }
public void LockThis()
{
lock (this)
{
System.Threading.Thread.Sleep(10000);
}
}
}
class Program
{
static void Main(string[] args)
{
var nancy = new Person {Name = "Nancy Drew", Age = 15};
var a = new Thread(nancy.LockThis);
a.Start();
var b = new Thread(Timewarp);
b.Start(nancy);
Thread.Sleep(10);
var anotherNancy = new Person { Name = "Nancy Drew", Age = 50 };
var c = new Thread(NameChange);
c.Start(anotherNancy);
a.Join();
Console.ReadLine();
}
static void Timewarp(object subject)
{
var person = subject as Person;
if (person == null) throw new ArgumentNullException("subject");
// A lock does not make the object read-only.
lock (person.Name)
{
while (person.Age <= 23)
{
// There will be a lock on 'person' due to the LockThis method running in another thread
if (Monitor.TryEnter(person, 10) == false)
{
Console.WriteLine("'this' person is locked!");
}
else Monitor.Exit(person);
person.Age++;
if(person.Age == 18)
{
// Changing the 'person.Name' value doesn't change the lock...
person.Name = "Nancy Smith";
}
Console.WriteLine("{0} is {1} years old.", person.Name, person.Age);
}
}
}
static void NameChange(object subject)
{
var person = subject as Person;
if (person == null) throw new ArgumentNullException("subject");
// You should avoid locking on strings, since they are immutable.
if (Monitor.TryEnter(person.Name, 30) == false)
{
Console.WriteLine("Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string "Nancy Drew".");
}
else Monitor.Exit(person.Name);
if (Monitor.TryEnter("Nancy Drew", 30) == false)
{
Console.WriteLine("Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining!");
}
else Monitor.Exit("Nancy Drew");
if (Monitor.TryEnter(person.Name, 10000))
{
string oldName = person.Name;
person.Name = "Nancy Callahan";
Console.WriteLine("Name changed from '{0}' to '{1}'.", oldName, person.Name);
}
else Monitor.Exit(person.Name);
}
}
Console output
'this' person is locked!
Nancy Drew is 16 years old.
'this' person is locked!
Nancy Drew is 17 years old.
Failed to obtain lock on 50 year old Nancy, because Timewarp(object) locked on string "Nancy Drew".
'this' person is locked!
Nancy Smith is 18 years old.
'this' person is locked!
Nancy Smith is 19 years old.
'this' person is locked!
Nancy Smith is 20 years old.
Failed to obtain lock using 'Nancy Drew' literal, locked by 'person.Name' since both are the same object thanks to inlining!
'this' person is locked!
Nancy Smith is 21 years old.
'this' person is locked!
Nancy Smith is 22 years old.
'this' person is locked!
Nancy Smith is 23 years old.
'this' person is locked!
Nancy Smith is 24 years old.
Name changed from 'Nancy Drew' to 'Nancy Callahan'.