为什么锁定我们要改变的对象是一种不好的做法？

Question

为什么在下面的代码中使用lock是一种不好的做法,我假设这是一个不好的做法,基于这个SO问题的答案

private void DoSomethingUseLess()
{
    List<IProduct> otherProductList = new List<IProduct>();
    Parallel.ForEach(myOriginalProductList, product =>
        {
           //Some code here removed for brevity
           //Some more code here :)
            lock (otherProductList)
            {
                otherProductList.Add((IProduct)product.Clone());
            }
        });
}

那边的答案提到这是不好的做法,但他们没有说明原因

注意:请忽略代码的用处,这只是为了示例目的,我知道它根本没用

Answer 1

从这里的C#语言参考:

通常,避免锁定公共类型或超出代码控制范围的实例.常见结构lock (this),lock (typeof (MyType))并lock ("myLock")违反本指南:

lock (this) 如果可以公开访问实例,则会出现问题.

lock (typeof (MyType))如果MyType可公开访问则是一个问题.

lock("myLock") 是一个问题,因为进程中使用相同字符串的任何其他代码将共享相同的锁.

最佳做法是定义要锁定的私有对象,或私有静态对象变量以保护所有实例共有的数据.

在你的情况下,我会阅读上面的指导,建议锁定你将要修改的集合是不好的做法.例如,如果您编写此代码:

lock (otherProductList) 
{
    otherProductList = new List<IProduct>(); 
}

......那么你的锁将毫无价值.由于这些原因,建议使用专用object变量进行锁定.

请注意,这并不意味着如果您使用您发布的代码,您的应用程序将会中断."最佳实践"通常被定义为提供技术上更具弹性的易于重复的模式.也就是说,如果你遵循最佳实践经验,并有一个专门的"锁定对象,"你是极不可能永远写断lock为基础的代码; 如果你不遵循最佳实践,也许一百次,你会被一个容易避免的问题所困扰.

此外(更一般地说),使用最佳实践编写的代码通常更容易修改,因为您可以避免意外的副作用.

Answer 2

这确实可能不是一个好主意，因为如果其他人使用相同的对象引用来执行 a lock，则可能会出现死锁。如果您的锁定对象有可能在您自己的代码之外可以访问，那么其他人可能会破坏您的代码。

根据您的代码想象以下示例：

namespace ClassLibrary1
{
    public class Foo : IProduct
    {
    }

    public interface IProduct
    {
    }

    public class MyClass
    {
        public List<IProduct> myOriginalProductList = new List<IProduct> { new Foo(), new Foo() };

        public void Test(Action<IEnumerable<IProduct>> handler)
        {
            List<IProduct> otherProductList = new List<IProduct> { new Foo(), new Foo() };
            Parallel.ForEach(myOriginalProductList, product =>
            {
                lock (otherProductList)
                {
                    if (handler != null)
                    {
                        handler(otherProductList);
                    }

                    otherProductList.Add(product);
                }
            });
        }
    }
}

现在，您编译您的库，将其发送给客户，该客户在他的代码中写入：

public class Program
{
    private static void Main(string[] args)
    {
        new MyClass().Test(z => SomeMethod(z));
    }

    private static void SomeMethod(IEnumerable<IProduct> myReference)
    {
        Parallel.ForEach(myReference, item =>
        {
            lock (myReference)
            {
                // Some stuff here
            }
        });
    }
}

那么对于您的客户来说，可能会出现一个难以调试的死锁，两个使用的线程中的每一个都在等待实例otherProductList不再被锁定。

我同意，这种情况不太可能发生，但它说明，如果您锁定的引用以任何可能的方式在您不拥有的一段代码中可见，那么最终代码就有可能被破坏。