C# 为什么 lock(this) {...} 不好？

Why is lock(this) {...} bad?

提问by Anton

The MSDN documentationsays that

在MSDN文档说，

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

is "a problem if the instance can be accessed publicly". I'm wondering why? Is it because the lock will be held longer than necessary? Or is there some more insidious reason?

是“如果可以公开访问实例的问题”。我想知道为什么？是因为锁的持有时间超过了必要的时间吗？还是有更阴险的原因？

采纳答案by Esteban Brenes

It is bad form to use thisin lock statements because it is generally out of your control who else might be locking on that object.

this在 lock 语句中使用这种形式是不好的，因为通常你无法控制还有谁可能会锁定该对象。

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 thisviolates encapsulation by exposing part of your locking implementation to the public. It is also not clear that you will be acquiring a lock on thisunless it has been documented. Even then, relying on documentation to prevent a problem is sub-optimal.

私有字段通常是更好的选择，因为编译器会对其实施访问限制，并且会封装锁定机制。使用this通过向公众公开部分锁定实现来违反封装。this除非已记录在案，否则您是否将获得锁定也不清楚。即便如此，依靠文档来预防问题也不是最佳选择。

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 lockmerely serves as a key. If a lock is already being held on that key, the lock cannot be made; otherwise, the lock is allowed.

最后，还有一个常见的误解，即lock(this)实际上修改了作为参数传递的对象，并以某种方式使其成为只读或不可访问。这是假的。作为参数传递给的对象lock仅用作键。如果该钥匙上已经有锁，则无法进行锁定；否则，允许锁定。

This is why it's bad to use strings as the keys in lockstatements, since they are immutable and are shared/accessible across parts of the application. You should use a private variable instead, an Objectinstance will do nicely.

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

Run the following C# code as an example.

运行以下 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);
    }
}

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

回答by Orion Edwards

Because if people can get at your object instance (ie: your this) pointer, then they can also try to lock that same object. Now they might not be aware that you're locking on thisinternally, so this may cause problems (possibly a deadlock)

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

In addition to this, it's also bad practice, because it's locking "too much"

除此之外，这也是不好的做法，因为它锁定“太多”

For example, you might have a member variable of List<int>, and the only thing you actually need to lock is that member variable. If you lock the entire object in your functions, then other things which call those functions will be blocked waiting for the lock. If those functions don't need to access the member list, you'll be causing other code to wait and slow down your application for no reason at all.

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

回答by Alan

...and the exact same arguments apply to this construct as well:

...完全相同的参数也适用于这个构造：

lock(typeof(SomeObject))

回答by crashmstr

Take a look at the MSDN Topic Thread Synchronization (C# Programming Guide)

看一下 MSDN 主题线程同步（C# 编程指南）

Generally, it is best to avoid locking on a public type, or on object instances beyond the control of your application. For example, lock(this) can be problematic if the instance can be accessed publicly, because code beyond your control may lock on the object as well. This could create deadlock situations where two or more threads wait for the release of the same object. Locking on a public data type, as opposed to an object, can cause problems for the same reason. Locking on literal strings is especially risky because literal strings are interned by the common language runtime (CLR). This means that there is one instance of any given string literal for the entire program, the exact same object represents the literal in all running application domains, on all threads. As a result, a lock placed on a string with the same contents anywhere in the application process locks all instances of that string in the application. As a result, it is best to lock a private or protected member that is not interned. Some classes provide members specifically for locking. The Array type, for example, provides SyncRoot. Many collection types provide a SyncRoot member as well.

通常，最好避免锁定公共类型或超出应用程序控制范围的对象实例。例如，如果实例可以公开访问，则 lock(this) 可能会出现问题，因为超出您控制范围的代码也可能锁定对象。这可能会造成死锁情况，其中两个或多个线程等待释放同一对象. 由于相同的原因，锁定公共数据类型而不是对象可能会导致问题。锁定文字字符串尤其危险，因为文字字符串由公共语言运行时 (CLR) 驻留。这意味着整个程序的任何给定字符串文字都有一个实例，完全相同的对象代表所有线程上所有正在运行的应用程序域中的文字。因此，在应用程序进程中的任何位置放置在具有相同内容的字符串上的锁会锁定应用程序中该字符串的所有实例。因此，最好锁定未实习的私有或受保护成员。一些类提供专门用于锁定的成员。例如，Array 类型提供 SyncRoot。许多集合类型也提供 SyncRoot 成员。

回答by Jason Hymanson

Because any chunk of code that can see the instance of your class can also lock on that reference. You want to hide (encapsulate) your locking object so that only code that needs to reference it can reference it. The keyword this refers to the current class instance, so any number of things could have reference to it and could use it to do thread synchronization.

因为可以看到您的类的实例的任何代码块也可以锁定该引用。您想隐藏（封装）您的锁定对象，以便只有需要引用它的代码才能引用它。关键字 this 指的是当前的类实例，因此任何数量的事物都可以引用它并可以使用它来进行线程同步。

To be clear, this is bad because some other chunk of code could use the class instance to lock, and might prevent your code from obtaining a timely lock or could create other thread sync problems. Best case: nothing else uses a reference to your class to lock. Middle case: something uses a reference to your class to do locks and it causes performance problems. Worst case: something uses a reference of your class to do locks and it causes really bad, really subtle, really hard-to-debug problems.

需要明确的是，这是不好的，因为其他一些代码块可能会使用类实例来锁定，并且可能会阻止您的代码及时获得锁定或可能会产生其他线程同步问题。最好的情况：没有其他东西使用对你的类的引用来锁定。中间情况：某些东西使用对你的类的引用来做锁，这会导致性能问题。最坏的情况：某些东西使用你的类的引用来做锁，这会导致非常糟糕、非常微妙、非常难以调试的问题。

回答by Bob Nadler

There's also some good discussion about this here: Is this the proper use of a mutex?

这里也有一些很好的讨论：这是互斥锁的正确使用吗？

回答by SOReader

Sorry guys but I can't agree with the argument that locking this might cause deadlock. You are confusing two things: deadlocking and starving.

抱歉，伙计们，我不同意锁定这可能会导致死锁的论点。你混淆了两件事：死锁和饥饿。

• You cannot cancel deadlock without interrupting one of the threads so after you get into a deadlock you cannot get out
• Starving will end automatically after one of the threads finishes its job

• 您无法在不中断其中一个线程的情况下取消死锁，因此在陷入死锁后您无法退出
• 在其中一个线程完成其工作后，饥饿将自动结束

Hereis a picture which illustrates the difference.

这是一张说明差异的图片。

Conclusion
You can still safely use lock(this)if thread starvation is not an issue for you. You still have to keep in mind that when the thread, which is starving thread using lock(this)ends in a lock having your object locked, it will finally end in eternal starvation ;)

结论如果线程饥饿对您来说不是问题，您
仍然可以安全地使用lock(this)。您仍然必须记住，当线程（正在使用的饥饿线程）lock(this)以锁定对象的锁结束时，它最终将永远处于饥饿状态；）

回答by Craig

I know this is an old thread, but because people can still look this up and rely on it, it seems important to point out that lock(typeof(SomeObject))is significantly worse than lock(this). Having said that; sincere kudos to Alan for pointing out that lock(typeof(SomeObject))is bad practice.

我知道这是一个旧线程，但是因为人们仍然可以查找并依赖它，所以指出它lock(typeof(SomeObject))比lock(this). 话说回来; 真诚地感谢 Alan 指出这lock(typeof(SomeObject))是不好的做法。

An instance of System.Typeis one of the most generic, coarse-grained objects there is. At the very least, an instance of System.Type is global to an AppDomain, and .NET can run multiple programs in an AppDomain. This means that two entirely different programs could potentially cause interference in one another even to the extent of creating a deadlock if they both try to get a synchronization lock on the same type instance.

的实例System.Type是最通用的粗粒度对象之一。至少，System.Type 的实例对于 AppDomain 是全局的，并且 .NET 可以在 AppDomain 中运行多个程序。这意味着如果两个完全不同的程序都试图在同一类型实例上获得同步锁，则它们可能会相互干扰，甚至会造成死锁。

So lock(this)isn't particularly robust form, can cause problems and should always raise eyebrows for all the reasons cited. Yet there is widely used, relatively well-respected and apparently stable code like log4net that uses the lock(this) pattern extensively, even though I would personally prefer to see that pattern change.

所以lock(this)不是特别健壮的形式，可能会导致问题，并且由于所有引用的原因应该总是引起注意。然而，有广泛使用、相对受人尊敬且明显稳定的代码，例如广泛使用 lock(this) 模式的 log4net，尽管我个人更希望看到这种模式发生变化。

But lock(typeof(SomeObject))opens up a whole new and enhanced can of worms.

但是lock(typeof(SomeObject))打开了一个全新的和增强的蠕虫罐头。

For what it's worth.

物有所值。

回答by William

There will be a problem if the instance can be accessed publicly because there could be other requests that might be using the same object instance. It's better to use private/static variable.

如果可以公开访问该实例，则会出现问题，因为可能有其他请求可能正在使用相同的对象实例。最好使用私有/静态变量。

回答by Raj Rao

Here is some sample code that is simpler to follow (IMO): (Will work in LinqPad, reference following namespaces: System.Net and System.Threading.Tasks)

这是一些更易于遵循的示例代码（IMO）：（将在LinqPad 中工作，参考以下命名空间：System.Net 和 System.Threading.Tasks）

Something to remember is that lock(x) basically is syntactic sugar and what it does is to use Monitor.Enter and then uses a try, catch, finally block to call Monitor.Exit. See: https://docs.microsoft.com/en-us/dotnet/api/system.threading.monitor.enter(remarks section)

需要记住的是，lock(x) 基本上是语法糖，它的作用是使用 Monitor.Enter，然后使用 try、catch、finally 块来调用 Monitor.Exit。请参阅：https: //docs.microsoft.com/en-us/dotnet/api/system.threading.monitor.enter（备注部分）

or use the C# lock statement (SyncLock statement in Visual Basic), which wraps the Enter and Exit methods in a try…finally block.

或使用 C# lock 语句（Visual Basic 中的 SyncLock 语句），它将 Enter 和 Exit 方法包装在 try...finally 块中。

void Main()
{
    //demonstrates why locking on THIS is BADD! (you should never lock on something that is publicly accessible)
    ClassTest test = new ClassTest();
    lock(test) //locking on the instance of ClassTest
    {
        Console.WriteLine($"CurrentThread {Thread.CurrentThread.ManagedThreadId}");
        Parallel.Invoke(new Action[]
        {
            () => {
                //this is there to just use up the current main thread. 
                Console.WriteLine($"CurrentThread {Thread.CurrentThread.ManagedThreadId}");
                },
            //none of these will enter the lock section.
            () => test.DoWorkUsingThisLock(1),//this will dead lock as lock(x) uses Monitor.Enter
            () => test.DoWorkUsingMonitor(2), //this will not dead lock as it uses Montory.TryEnter
        });
    }
}

public class ClassTest
{
    public void DoWorkUsingThisLock(int i)
    {
        Console.WriteLine($"Start ClassTest.DoWorkUsingThisLock {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
        lock(this) //this can be bad if someone has locked on this already, as it will cause it to be deadlocked!
        {
            Console.WriteLine($"Running: ClassTest.DoWorkUsingThisLock {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
            Thread.Sleep(1000);
        }
        Console.WriteLine($"End ClassTest.DoWorkUsingThisLock Done {i}  CurrentThread {Thread.CurrentThread.ManagedThreadId}");
    }

    public void DoWorkUsingMonitor(int i)
    {
        Console.WriteLine($"Start ClassTest.DoWorkUsingMonitor {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
        if (Monitor.TryEnter(this))
        {
            Console.WriteLine($"Running: ClassTest.DoWorkUsingMonitor {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
            Thread.Sleep(1000);
            Monitor.Exit(this);
        }
        else
        {
            Console.WriteLine($"Skipped lock section!  {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
        }

        Console.WriteLine($"End ClassTest.DoWorkUsingMonitor Done {i} CurrentThread {Thread.CurrentThread.ManagedThreadId}");
        Console.WriteLine();
    }
}

Output

输出

CurrentThread 15
CurrentThread 15
Start ClassTest.DoWorkUsingMonitor 2 CurrentThread 13
Start ClassTest.DoWorkUsingThisLock 1 CurrentThread 12
Skipped lock section!  2 CurrentThread 13
End ClassTest.DoWorkUsingMonitor Done 2 CurrentThread 13

Notice that Thread#12 never ends as its dead locked.

请注意，线程#12 永远不会因为死锁而结束。