49

次のコードのようにロックを使用するのはなぜ悪い習慣なのですか?私はこれがこの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());
            }
        });
}

あちらの答えは、それが悪い習慣であると述べていますが、理由は述べていません

注：コードの有用性を無視してください。これは単なる例であり、まったく役に立たないことはわかっています

4

2 に答える 2

47

ここのC＃言語リファレンスから：

一般に、パブリック型、またはコードの制御を超えたインスタンスのロックは避けてください。一般的な構文lock (this)、lock (typeof (MyType))、およびは、次のlock ("myLock")ガイドラインに違反しています。

lock (this)インスタンスがパブリックにアクセスできる場合は問題です。

lock (typeof (MyType))パブリックにアクセスできる場合は問題ですMyType。

lock("myLock")同じ文字列を使用するプロセス内の他のコードが同じロックを共有するため、これは問題です。

ベスト プラクティスは、プライベート オブジェクトを定義してロックするか、プライベートな静的オブジェクト変数を定義して、すべてのインスタンスに共通のデータを保護することです。

あなたの場合、上記のガイダンスを読んで、変更するコレクションをロックすることは悪い習慣であることを示唆しています。たとえば、次のコードを記述したとします。

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

...その場合、ロックは無価値になります。objectこれらの理由から、ロック専用の変数を使用することをお勧めします。

これは、投稿したコードを使用した場合にアプリケーションが壊れるという意味ではないことに注意してください。通常、「ベスト プラクティス」は、技術的に回復力のある簡単に繰り返されるパターンを提供するために定義されます。つまり、ベスト プラクティスに従い、専用の「ロック オブジェクト」を使用する場合、壊れたに基づくコードを作成する可能性はほとんどありません。lockベスト プラクティスに従わないと、おそらく 100 分の 1 の確率で、簡単に回避できる問題に悩まされることになります。

さらに (そしてより一般的に)、ベスト プラクティスを使用して記述されたコードは、通常、予期しない副作用に対する警戒心が少なくなるため、より簡単に変更できます。

于 2012-08-02T10:11:34.267 に答える
4

他の誰かが同じオブジェクト参照を使用して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次に、インスタンスがロックされなくなるのを待っている 2 つの使用済みスレッドのそれぞれで、顧客にとってデバッグが困難なデッドロックが発生する可能性があります。

このシナリオが発生する可能性は低いことに同意しますが、ロックされた参照が所有していないコードの一部に表示されている場合、最終的なコードが破損する可能性があることを示しています。

于 2012-08-02T10:14:52.737 に答える