c#-为什么锁定要更改的对象是一种不好的做法?

为什么在下面的代码中使用锁是一种不好的做法,基于此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());
            }
        });
}

那里的答案提到这是不好的作法,但他们没有说为什么

注意:请忽略代码的有用性,这仅出于示例目的,我知道它根本没有用

Vamsi asked 2020-08-01T09:55:07Z
2个解决方案
42 votes

从此处的C#语言参考:

通常,避免锁定公共类型或代码无法控制的实例。 常见构造lockMyTypelock ("myLock")违反了该准则:

如果可以公开访问该实例,则lock会出现问题。

如果MyType可公开访问,则lock是个问题。

lock是一个问题,因为该过程中还有其他任何代码 使用相同的字符串,将共享相同的锁。

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

在您的情况下,我会读以上指导,因为这表明锁定将要修改的集合是不明智的做法。 例如,如果您编写此代码:

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

...那么您的锁将一文不值。 由于这些原因,建议使用专用的lock变量进行锁定。

请注意,这并不意味着如果您使用发布的代码,应用程序就会中断。 通常将“最佳实践”定义为提供易于重复的模式,这些模式在技术上更具弹性。 也就是说,如果您遵循最佳实践并拥有专用的“锁定对象”,则极不可能编写破损的基于lock的代码。 如果您不遵循最佳实践,那么可能会百分之一地被一个容易避免的问题所困扰。

此外(通常),使用最佳实践编写的代码通常更容易修改,因为您可以对意外的副作用有所警惕。

Dan Puzey answered 2020-08-01T09:56:08Z
3 votes

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

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

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实例不再被锁定。

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

ken2k answered 2020-08-01T09:56:46Z
translate from https://stackoverflow.com:/questions/11775205/why-is-it-a-bad-practice-to-lock-the-object-we-are-going-to-change