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

Question

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

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

score 535 · Accepted Answer

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

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

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

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

这就是为什么在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'.

score 69 · Accepted Answer

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

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

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

score 44 · Accepted Answer

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

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

score 36 · Accepted Answer

我知道这是一个旧线程，但是因为人们仍然可以查找并依赖它，所以指出它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))开辟了一个全新的和增强的蠕虫罐头。

物有所值。

score 27 · Accepted Answer

27

...并且完全相同的论点也适用于此构造：

lock(typeof(SomeObject))

于 2008-10-30T19:25:54.650 回答

score 10 · Accepted Answer

想象一下，您的办公室有一位熟练的秘书，他是部门的共享资源。有时，您会因为有任务而冲向他们，只是希望您的另一位同事尚未认领他们。通常你只需要等待一小段时间。

因为关怀是分享，所以您的经理决定客户也可以直接使用秘书。但这有一个副作用：客户甚至可能在您为该客户工作时认领他们，并且您还需要他们执行部分任务。发生死锁，因为声明不再是层次结构。如果一开始就不允许客户认领，本可以完全避免这种情况。

lock(this)正如我们所见，这很糟糕。外部对象可能会锁定该对象，并且由于您无法控制谁在使用该类，因此任何人都可以锁定它......这就是上面描述的确切示例。同样，解决方案是限制对象的曝光。但是，如果您有一个private,protected或internal类，您已经可以控制谁锁定了您的 object，因为您确定您自己编写了代码。所以这里的信息是：不要将其公开为public. 此外，确保在类似场景中使用锁可以避免死锁。

与此完全相反的是锁定在整个应用程序域中共享的资源——最坏的情况。这就像把你的秘书放在外面，让外面的每个人都可以认领他们。结果是彻底的混乱——或者就源代码而言：这是一个坏主意；扔掉它并重新开始。那么我们该怎么做呢？

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

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

score 4 · Accepted Answer

如果锁定共享资源，锁定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

score 3 · Accepted Answer

有一篇非常好的文章http://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objects由 Microsoft® .NET 运行时的性能架构师 Rico Mariani撰写

摘抄：

这里的基本问题是您不拥有类型对象，并且您不知道还有谁可以访问它。一般来说，依赖锁定一个不是您创建并且不知道还有谁可能正在访问的对象是一个非常糟糕的主意。这样做会导致僵局。最安全的方法是只锁定私有对象。

score 2 · Accepted Answer

2

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

于 2008-10-30T19:59:31.323 回答

score 1 · Accepted Answer

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

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

score 1 · Accepted Answer

对不起，伙计们，但我不同意锁定它可能会导致死锁的论点。您混淆了两件事：死锁和饥饿。

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

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

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

score 1 · Accepted Answer

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

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

或使用 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();
    }
}

输出

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

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

score 1 · Accepted Answer

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

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

所以解决方法是在类中添加一个私有对象，例如lockObject，并将代码区域放在lock语句里面，如下图：

lock (lockObject)
{
...
}

score 1 · Accepted Answer

这是一个更简单的说明（取自这里的问题 34）为什么 lock(this) 不好并且当您的类的使用者也尝试锁定对象时可能导致死锁。下面，三个线程中只有一个可以继续，另外两个处于死锁状态。

class SomeClass
{
    public void SomeMethod(int id)
    {
        **lock(this)**
        {
            while(true)
            {
                Console.WriteLine("SomeClass.SomeMethod #" + id);
            }
        }
    }
}

class Program
{
    static void Main(string[] args)
    {
        SomeClass o = new SomeClass();

        lock(o)
        {
            for (int threadId = 0; threadId < 3; threadId++)
            {
                Thread t = new Thread(() => {
                    o.SomeMethod(threadId);
                        });
                t.Start();
            }

            Console.WriteLine();
        }

为了解决这个问题，这个人使用了 Thread.TryMonitor（有超时）而不是锁：

            Monitor.TryEnter(temp, millisecondsTimeout, ref lockWasTaken);
            if (lockWasTaken)
            {
                doAction();
            }
            else
            {
                throw new Exception("Could not get lock");
            }

https://blogs.appbeat.io/post/c-how-to-lock-without-deadlocks

score 1 · Accepted Answer

您可以建立一个规则，说明一个类可以具有锁定“this”或类中的代码实例化的任何对象的代码。因此，如果不遵循模式，这只是一个问题。

如果您想保护自己免受不遵循此模式的代码的影响，那么接受的答案是正确的。但如果遵循模式，这不是问题。

lock(this) 的优点是效率。如果您有一个包含单个值的简单“值对象”怎么办。它只是一个包装器，它被实例化了数百万次。通过要求创建一个仅用于锁定的私有同步对象，您基本上使对象的大小翻了一番，分配的数量也翻了一番。当性能很重要时，这是一个优势。

当您不关心分配数量或内存占用时，由于其他答案中指出的原因，最好避免使用 lock(this) 。

score -1 · Accepted Answer

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

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

16 回答 16

Related

Reference