为什么lock(this) {...}是不好的？

Question

据MSDN文档所述

public class SomeObject
{
  public void SomeOperation()
  {
    lock(this)
    {
      //Access instance variables
    }
  }
}

如果实例可以公开访问，则会出现问题。我想知道为什么？是因为锁定时间比必要时间更长吗？还是有更阴险的原因？

- Anton

Answer 1

在锁定语句中使用this是不好的形式，因为通常情况下，你无法控制其他人是否正在锁定该对象。

为了正确规划并行操作，应特别注意考虑可能的死锁情况，并且具有未知数量的锁入口会妨碍此过程。例如，任何一个拥有该对象引用的人都可以在不让对象设计者/创建者知道的情况下对其进行锁定。这增加了多线程解决方案的复杂性，并可能影响其正确性。

私有字段通常是更好的选择，因为编译器将强制执行对其的访问限制，并封装锁定机制。使用this违反了封装原则，向公众暴露了部分锁定实现。除非已经记录，否则不清楚是否将在this上获取锁定。即使如此，依靠文档来防止问题也不是最佳选择。

最后，普遍存在这样一种误解，即lock(this)实际上修改了作为参数传递的对象，并以某种方式使其只读或无法访问。这是错误的。传递给lock的对象仅充当键。如果该键已经被锁定，则无法进行锁定；否则，锁定将被允许。

这就是为什么在lock语句中使用字符串作为键是不好的，因为它们是不可变的，并且在应用程序的各个部分之间共享/可访问。你应该使用一个私有变量，一个Object实例即可。

请运行以下C#代码作为示例。

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);
    }
}

控制台输出

'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'.

- Esteban Brenes

---

据我理解: (1) Nancy在thread1中使用lock(this)。**(2)** 同一个Nancy被锁定在thread1中，同时在thread2中变老了 - 证明一个被锁定的对象不是只读的。此外，**(2a)** 在thread2中，这个Nancy对象也被Name锁定。**(3)** 创建一个具有相同Name的不同对象。**(4)** 将其传入thread3并尝试使用Name进行锁定。**(大结局)** 但是，“字符串是不可变的”，这意味着引用字符串“Nancy Drew”的任何对象都在查看内存中完全相同的字符串实例。因此，当object1锁定相同的值时，object2不能对字符串进行锁定。 - radarbob

使用标准变量而不是 lock(this) 是标准建议；需要注意的是，这样做通常会使外部代码无法在方法调用之间保持与对象关联的锁。这可能是好事或坏事。允许外部代码持有锁的任意时间存在一定的风险，类应该通常被设计为使这种用法不必要，但并不总是有实际的替代方案。举个简单的例子，除非集合实现了自己的 ToArray 或 ToList 方法... - supercat

与 IEnumerable<T> 扩展方法相反，想要获取集合快照的线程可能只能在锁定所有更改的情况下对其进行枚举。为了做到这一点，它必须访问一个由任何可能会更改集合的代码所获取的锁。如果不公开该锁，则可能无法周期性地执行异步集合快照（例如更新集合浏览用户界面）。 - supercat

有一个常见的误解，即lock(this)实际上会修改传递作为参数的对象，并以某种方式使其变为只读或不可访问。这是错误的。我认为这些谈话是关于CLR对象中的SyncBlock位，因此从形式上讲这是正确的 - lock会修改对象本身。 - sll

@EstebanBrenes，感谢您的回复。我相信您没有解决我的问题。NameChange（..）方法中的最后一个Monitor.Exit（..）不应该是else结构的一部分，而应该是if（Monitor.TryEnter（..））结构的一部分。尽管如此，我还是感谢您的关注和文章，它让我大开眼界。 - AviFarah

Answer 2

如果人们可以访问您的对象实例（即this指针），那么他们也可以尝试锁定同一对象。现在他们可能不知道您正在内部锁定this，因此这可能会引起问题（可能会导致死锁）。

此外，这也是不良实践，因为它锁定了“太多”东西。

例如，您可能有一个List<int>成员变量，而您唯一需要锁定的是该成员变量。如果在函数中锁定整个对象，则调用这些函数的其他内容将被阻止等待锁定。如果这些函数不需要访问成员列表，那么您将导致其他代码无缘无故地等待并减慢应用程序的速度。

- Orion Edwards

---

这个答案的最后一段不正确。锁定并不会以任何方式使对象无法访问或只读。Lock(this) 并不会阻止另一个线程调用或修改该对象的引用。 - Esteban Brenes

如果被调用的其他方法也使用了 lock(this)，那么这个锁就会生效。我相信这就是他要表达的意思。请注意，“如果您在函数中锁定了整个对象”这句话。 - Herms

@Orion：这更清晰了。 @Herms：是的，但你不需要使用“this”来实现该功能，在列表中，SyncRoot属性可以实现此目的，例如，同时明确同步应在该键上完成。 - Esteban Brenes

回复：锁定“太多”：决定锁定什么是一项微妙的平衡。请注意，获取锁涉及缓存刷新CPU操作，而且有点昂贵。换句话说：不要锁定并更新每个单独的整数。 :) - Zan Lynx

@JoakimM.H. - 假设你有一个列表和一个整数，你想限制对它们的访问。如果它们不需要一起锁定（锁定过多），那么单个锁可能会成为问题，因此你可以选择使用两个锁。 - Orion Edwards

Answer 3

请查看 MSDN Topic 线程同步（C# 编程指南）。

通常最好避免在公共类型上或对应用程序无法控制的对象实例上进行锁定。例如，如果可以公开访问实例，则 lock(this) 可能存在问题，因为不受您控制的代码也可能在该对象上进行锁定。这可能会创建死锁情况，其中两个或多个线程等待释放相同的对象。锁定公共数据类型（而不是对象）也可能由于同样的原因导致问题。锁定文字字符串尤其危险，因为常规语言运行时（CLR）会对文字字符串进行整理（intern），这意味着对于整个程序，任何给定的字面值字符串都有一个实例，确切地说，在所有运行的应用程序域和所有线程中，同一个对象表示该字面值。结果，放置在应用程序中任何地方具有相同内容的字符串上的锁定都会锁定应用程序中该字符串的所有实例。因此，最好锁定未经整理的私有或受保护成员。一些类专门提供了用于锁定的成员。例如，Array 类提供 SyncRoot。许多集合类型也提供 SyncRoot 成员。

- crashmstr

Answer 4

我知道这是一个旧的帖子，但因为人们仍然可以查找并依靠它，所以指出lock(typeof(SomeObject))比lock(this)明显更糟糕是很重要的。话虽如此，真诚地向Alan表示敬意，他指出lock(typeof(SomeObject))是一种不好的实践。

System.Type的实例是最通用、最粗粒度的对象之一。至少，System.Type的一个实例对于一个AppDomain是全局的，.NET可以在一个AppDomain中运行多个程序。这意味着两个完全不同的应用程序可能会在全局的System.Type实例上互相干扰，甚至可能导致死锁。

因此，lock(this)不是特别健壮的形式，可能会引起问题，并且对于所有引用原因都应该引起注意。然而，像log4net这样广泛使用、相对受尊敬和表现稳定的代码仍然大量使用lock(this)模式，即使我个人更喜欢看到该模式改变。

但lock(typeof(SomeObject))会开启一个全新且增强的问题集。

就这些吧。

- Craig Tullis

Answer 5

...而且完全相同的论点也适用于这个结构：

lock(typeof(SomeObject))

- Alan

---

lock(typeof(SomeObject))比lock(this)要糟糕得多。 - Craig Tullis

好的，lock(Application.Current)甚至比lock(this)更糟糕，但是谁会尝试这些愚蠢的事情呢？lock(this)似乎很合理简洁，但这些其他例子却不是。 - Zar Shardan

我不认为lock(this)看起来特别逻辑和简洁。这是一种非常粗略的锁定方式，任何其他代码都可以在您的对象上锁定，可能会干扰您的内部代码。采用更细粒度的锁定，并假设 tighter control。lock(this)的好处是它比lock(typeof(SomeObject))要好得多。 - Craig Tullis

Answer 6

想象你在办公室里有一位熟练的秘书，是部门内的一个共享资源。偶尔你会赶向他们，因为你有一个任务，只希望你的另一个同事还没有占用他们。通常你只需要等待很短的时间。

因为关心就是分享，你的经理决定顾客也可以直接使用这个秘书。但是这有一个副作用：当你正在为这个顾客工作并且你也需要他们执行部分任务时，顾客甚至可能会占用他们。死锁发生了，因为声明不再是层次结构。这完全可以通过一开始不允许顾客声明来避免。

lock(this) 是不好的，正如我们所看到的。外部对象可能锁定该对象，由于您不控制谁在使用该类，任何人都可以锁定它... 这与上述描述的完全相同。解决方法仍然是限制对象的曝光度。但是，如果您有一个 private、protected 或 internal 类，您已经可以控制谁在锁定您的对象，因为您确信已经自己编写了代码。因此，这里的信息是：不要将其公开为 public。此外，在类似的情况下使用锁可以避免死锁。

完全相反的是，在应用程序域中锁定共享资源 - 最坏的情况。这就像把你的秘书放在外面，让那里的每个人都可以占用他们。结果是彻底的混乱 - 或者用源代码的术语来说：这是一个坏主意；抛弃它并重新开始。那么我们该怎么做呢？

正如大多数人在这里指出的那样，类型在应用程序域中是共享的。但是我们甚至可以使用更好的东西：字符串。原因是字符串是池化的。换句话说：如果您有两个具有在应用程序域中具有相同内容的字符串，则它们具有完全相同的指针的几率。由于指针用作锁键，您基本上得到了“为未定义的行为做好准备”的同义词。

同样，您不应该锁定WCF对象、HttpContext.Current、Thread.Current、Singletons（通常情况下），等等。避免所有这些的最简单方法是什么呢？private [static] object myLock = new object();

- atlaste

---

私有类并不能防止这个问题。外部代码可以获取到私有类的实例引用... - Rashack

@Rashack，虽然你在技术上是正确的（+1 表示指出了这一点），但我的观点是，你应该控制谁锁定实例。像那样返回实例会破坏这一点。 - atlaste

对于锁定来说，字符串可能比类型更糟糕，原因在于它们可能被“池化”或者内部化：即使这些共享对象没有以公开的方式创建，它们仍然存在。 - undefined

Answer 7

锁定this指针可能是有害的，如果您在锁定共享资源时使用。共享资源可以是静态变量或计算机上的文件 - 即，在类的所有用户之间共享的内容。原因是每次实例化类时，this指针将包含对内存中不同位置的引用。因此，在一个类的一个实例中锁定this与在另一个实例中锁定this是不同的。

请查看以下代码以了解我的意思。将以下代码添加到控制台应用程序中的主程序中：

    static void Main(string[] args)
    {
         TestThreading();
         Console.ReadLine();
    }

    public static void TestThreading()
    {
        Random rand = new Random();
        Thread[] threads = new Thread[10];
        TestLock.balance = 100000;
        for (int i = 0; i < 10; i++)
        {
            TestLock tl = new TestLock();
            Thread t = new Thread(new ThreadStart(tl.WithdrawAmount));
            threads[i] = t;
        }
        for (int i = 0; i < 10; i++)
        {
            threads[i].Start();
        }
        Console.Read();
    }

创建一个新的类，类似于下面的示例。

 class TestLock
{
    public static int balance { get; set; }
    public static readonly Object myLock = new Object();

    public void Withdraw(int amount)
    {
      // Try both locks to see what I mean
      //             lock (this)
       lock (myLock)
        {
            Random rand = new Random();
            if (balance >= amount)
            {
                Console.WriteLine("Balance before Withdrawal :  " + balance);
                Console.WriteLine("Withdraw        : -" + amount);
                balance = balance - amount;
                Console.WriteLine("Balance after Withdrawal  :  " + balance);
            }
            else
            {
                Console.WriteLine("Can't process your transaction, current balance is :  " + balance + " and you tried to withdraw " + amount);
            }
        }

    }
    public void WithdrawAmount()
    {
        Random rand = new Random();
        Withdraw(rand.Next(1, 100) * 100);
    }
}

这里是程序在此处锁定的运行。

   Balance before Withdrawal :  100000
    Withdraw        : -5600
    Balance after Withdrawal  :  94400
    Balance before Withdrawal :  100000
    Balance before Withdrawal :  100000
    Withdraw        : -5600
    Balance after Withdrawal  :  88800
    Withdraw        : -5600
    Balance after Withdrawal  :  83200
    Balance before Withdrawal :  83200
    Withdraw        : -9100
    Balance after Withdrawal  :  74100
    Balance before Withdrawal :  74100
    Withdraw        : -9100
    Balance before Withdrawal :  74100
    Withdraw        : -9100
    Balance after Withdrawal  :  55900
    Balance after Withdrawal  :  65000
    Balance before Withdrawal :  55900
    Withdraw        : -9100
    Balance after Withdrawal  :  46800
    Balance before Withdrawal :  46800
    Withdraw        : -2800
    Balance after Withdrawal  :  44000
    Balance before Withdrawal :  44000
    Withdraw        : -2800
    Balance after Withdrawal  :  41200
    Balance before Withdrawal :  44000
    Withdraw        : -2800
    Balance after Withdrawal  :  38400

以下是程序在myLock上锁定的运行。

Balance before Withdrawal :  100000
Withdraw        : -6600
Balance after Withdrawal  :  93400
Balance before Withdrawal :  93400
Withdraw        : -6600
Balance after Withdrawal  :  86800
Balance before Withdrawal :  86800
Withdraw        : -200
Balance after Withdrawal  :  86600
Balance before Withdrawal :  86600
Withdraw        : -8500
Balance after Withdrawal  :  78100
Balance before Withdrawal :  78100
Withdraw        : -8500
Balance after Withdrawal  :  69600
Balance before Withdrawal :  69600
Withdraw        : -8500
Balance after Withdrawal  :  61100
Balance before Withdrawal :  61100
Withdraw        : -2200
Balance after Withdrawal  :  58900
Balance before Withdrawal :  58900
Withdraw        : -2200
Balance after Withdrawal  :  56700
Balance before Withdrawal :  56700
Withdraw        : -2200
Balance after Withdrawal  :  54500
Balance before Withdrawal :  54500
Withdraw        : -500
Balance after Withdrawal  :  54000

- ItsAllABadJoke

---

你的例子中需要注意的是什么，比如你展示了哪些不正确的内容。当你使用Random rand = new Random();时很难发现错误在哪里。我想我看到了，它是重复的余额。 - Seabizkit

Answer 8

有一篇关于此的非常好的文章http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects，作者是微软® .NET运行时的性能架构师Rico Mariani。

摘录：

基本问题在于您不拥有类型对象，也不知道谁还会访问它。通常情况下，依赖于锁定一个您没有创建并且不知道谁可能访问的对象是非常糟糕的想法。这样做会引发死锁。最安全的方法是仅锁定私有对象。

- Vikrant

Answer 9

这里也有一些关于这个问题的好讨论：Is this the proper use of a mutex?

- Bob Nadler

Answer 10

请参考以下链接，了解为什么锁定 (this) 不是一个好主意。

https://learn.microsoft.com/en-us/dotnet/standard/threading/managed-threading-best-practices

因此解决方案是向类中添加一个私有对象，例如 lockObject，并在锁语句内部放置代码区域，如下所示：

lock (lockObject)
{
...
}

- Dhruv Rangunwala