520

MSDNのドキュメントには、

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

「インスタンスがパブリックにアクセスできる場合の問題」です。なぜだろう？ロックが必要以上に長く保持されるためですか？それとも、もっと陰湿な理由がありますか？

4

16 に答える 16

535

lock ステートメントで使用するのは不適切な形式thisです。一般に、そのオブジェクトを他の誰がロックしているかは制御できないためです。

並列操作を適切に計画するには、デッドロックの可能性を考慮して特別な注意を払う必要があります。ロック エントリ ポイントの数が不明な場合は、これが妨げられます。たとえば、オブジェクトへの参照を持つ人は、オブジェクトの設計者/作成者が知らないうちにロックできます。これにより、マルチスレッド ソリューションの複雑さが増し、その正確性に影響する可能性があります。

プライベート フィールドは通常、コンパイラによってアクセス制限が適用され、ロック メカニズムがカプセル化されるため、より適切なオプションです。使用thisすると、ロックの実装の一部が公開され、カプセル化に違反します。また、文書化されていない限り、ロックオンを取得するかどうかも明確ではありませんthis。それでも、ドキュメントに依存して問題を回避することは最適ではありません。

lock(this)最後に、パラメータとして渡されたオブジェクトを実際に変更し、何らかの方法でオブジェクトを読み取り専用またはアクセス不能にするという一般的な誤解があります。これは誤りです。パラメータとして渡されたオブジェクトは、lock単にキーとして機能します。そのキーですでにロックが保持されている場合、ロックを作成することはできません。それ以外の場合、ロックは許可されます。

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'.
于 2008-10-30T20:34:15.980 に答える
69

人々があなたのオブジェクト インスタンス (つまり、あなたのthis) ポインターを取得できる場合、同じオブジェクトをロックしようとすることもできるからです。現在、彼らはあなたがthis内部的にロックしていることを認識していない可能性があるため、問題が発生する可能性があります (デッドロックの可能性があります) 。

これに加えて、ロックが「多すぎる」ため、悪い習慣でもあります。

たとえば、メンバー変数がありList<int>、実際にロックする必要があるのはそのメンバー変数だけです。関数内のオブジェクト全体をロックすると、それらの関数を呼び出す他のものはロックを待ってブロックされます。これらの関数がメンバー リストにアクセスする必要がない場合、他のコードを待機させ、理由もなくアプリケーションの速度を低下させることになります。

于 2008-10-30T19:22:27.653 に答える
44

MSDN トピックスレッド同期 (C# プログラミング ガイド)を参照してください。

一般に、パブリック型、またはアプリケーションの制御が及ばないオブジェクト インスタンスに対するロックは避けるのが最善です。たとえば、lock(this) は、インスタンスがパブリックにアクセスできる場合に問題になる可能性があります。これは、制御できないコードがオブジェクトをロックする可能性があるためです。これにより、2 つ以上のスレッドが同じオブジェクトの解放を待機するデッドロック状態が発生する可能性があります。. オブジェクトではなくパブリック データ型をロックすると、同じ理由で問題が発生する可能性があります。リテラル文字列は共通言語ランタイム (CLR) によってインターンされるため、リテラル文字列のロックは特に危険です。これは、プログラム全体に対して特定の文字列リテラルのインスタンスが 1 つ存在することを意味します。まったく同じオブジェクトが、すべてのスレッドで、実行中のすべてのアプリケーション ドメインでリテラルを表します。その結果、アプリケーション プロセスのどこかにある同じ内容の文字列にロックを設定すると、アプリケーション内のその文字列のすべてのインスタンスがロックされます。そのため、インターンされていないプライベートまたは保護されたメンバーをロックすることをお勧めします。一部のクラスは、ロック専用のメンバーを提供します。たとえば、Array 型は SyncRoot を提供します。多くのコレクション型は、SyncRoot メンバーも提供します。

于 2008-10-30T19:27:10.153 に答える
36

lock(typeof(SomeObject))これが古いスレッドであることはわかっていますが、人々はまだこれを調べて信頼できるため、 がよりも大幅に悪いことを指摘することが重要と思われlock(this)ます。そうは言っても; lock(typeof(SomeObject))それが悪い習慣であることを指摘してくれたAlanに心からの称賛を。

のインスタンスはSystem.Type、最も一般的で粒度の粗いオブジェクトの 1 つです。少なくとも、System.Type のインスタンスは AppDomain に対してグローバルであり、.NET は AppDomain で複数のプログラムを実行できます。これは、2 つのまったく異なるアプリケーションが、両方とも System.Type の同じグローバル インスタンスで同期ロックを取得しようとすると、デッドロックを作成する程度まで、相互に干渉を引き起こす可能性があることを意味します。

そのlock(this)ため、特に堅牢な形式ではなく、問題を引き起こす可能性があり、引用されたすべての理由で常に眉を上げる必要があります. それでも、log4net のように、lock(this) パターンを広範囲に使用する、広く使用され、比較的尊敬され、明らかに安定しているコードがあります。

しかしlock(typeof(SomeObject))、まったく新しい強化されたワームの缶が開かれます。

それが価値があるもののために。

于 2012-05-09T06:08:41.027 に答える
27

...そして、まったく同じ引数がこの構成にも適用されます。

lock(typeof(SomeObject))
于 2008-10-30T19:25:54.650 に答える
10

あなたのオフィスには、部門内の共有リソースである熟練した秘書がいると想像してください。ときどき、仕事があるために急いで彼らに向かいますが、他の同僚がまだそれらを要求していないことを願っています。通常、短時間だけお待ちいただく必要があります。

ケアリングは分かち合いであるため、マネージャーは、顧客が秘書を直接使用することもできると判断します。しかし、これには副作用があります。あなたがこの顧客のために働いている間に、顧客がそれらを要求することさえあり、タスクの一部を実行するためにも顧客が必要になる場合があります。請求が階層でなくなるため、デッドロックが発生します。これは、顧客が最初にそれらを請求できないようにすることで、完全に回避できた可能性があります.

lock(this)私たちが見てきたように悪いです。外部オブジェクトがオブジェクトをロックする可能性があり、クラスを使用しているユーザーを制御できないため、誰でもロックできます...上記の正確な例です。繰り返しますが、解決策はオブジェクトの露出を制限することです。ただし、またはクラスがある場合はprivate、protectedオブジェクトinternalをロックしている人を既に制御できます。これは、コードを自分で作成したことが確実であるためです。したがって、ここでのメッセージは次のとおりですpublic。また、同様のシナリオでロックが使用されるようにすることで、デッドロックを回避できます。

これとはまったく逆に、アプリ ドメイン全体で共有されているリソースをロックすることです。これは最悪のシナリオです。それは、あなたの秘書を外に出して、そこにいる誰もが秘書を主張できるようにするようなものです. その結果、まったくの混乱が生じます。つまり、ソース コードに関して言えば、それは悪い考えでした。捨ててやり直す。では、どうすればよいのでしょうか。

ここでほとんどの人が指摘しているように、型はアプリ ドメインで共有されます。しかし、もっと良いものがあります: 文字列です。その理由は、文字列がプールされているためです。つまり、アプリ ドメインに同じ内容を持つ 2 つの文字列がある場合、それらがまったく同じポインターを持つ可能性があります。ポインタはロックキーとして使用されるため、基本的に得られるのは「未定義の動作に備える」の同義語です。

同様に、WCF オブジェクト、HttpContext.Current、Thread.Current、Singletons (一般的に) などをロックするべきではありません。これらすべてを回避する最も簡単な方法は?private [static] object myLock = new object();

于 2013-06-12T09:35:08.260 に答える
3

Microsoft® .NE​​T ランタイムのパフォーマンス アーキテクトである Rico Mariani によるhttp://bytes.com/topic/c-sharp/answers/249277-dont-lock-type-objectsに関する非常に優れた記事があります。

抜粋：

ここでの基本的な問題は、あなたが型オブジェクトを所有しておらず、他の誰がそれにアクセスできるかわからないことです。一般に、自分が作成したのではないオブジェクトをロックすることに頼り、他に誰がアクセスしているのかわからないというのは、非常に悪い考えです。デッドロックを招きます。最も安全な方法は、プライベート オブジェクトのみをロックすることです。

于 2014-08-14T23:22:49.367 に答える
2

これについての良い議論もここにあります: Is this the appropriate use of a mutex?

于 2008-10-30T19:59:31.323 に答える
1

申し訳ありませんが、これをロックするとデッドロックが発生する可能性があるという議論には同意できません。デッドロックと飢餓という 2 つのことを混同しています。

  • スレッドの1つを中断せずにデッドロックをキャンセルすることはできないため、デッドロックに入った後は抜け出せません
  • いずれかのスレッドがジョブを終了すると、飢餓状態は自動的に終了します

これが違いを示す写真です。

結論スレッドの枯渇が問題にならない場合
でも、安全に使用できます。lock(this)使用している飢餓スレッドであるスレッドlock(this)が、オブジェクトがロックされているロックで終了すると、最終的に永遠の飢餓で終了することに注意する必要があります;)

于 2012-03-26T10:59:01.050 に答える
1

以下は、従うのが簡単なサンプルコードです（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

Thread#12 は完全にロックされているため、決して終了しないことに注意してください。

于 2013-06-10T20:09:16.240 に答える
1

ロック (これ) が良くない理由を説明している次のリンクを参照してください。

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

そのため、解決策は、lockObject などのプライベート オブジェクトをクラスに追加し、以下に示すように lock ステートメント内にコード領域を配置することです。

lock (lockObject)
{
...
}
于 2014-12-03T18:47:56.883 に答える
1

クラスのインスタンスを見ることができるコードのチャンクも、その参照をロックできるためです。ロック オブジェクトを非表示 (カプセル化) して、それを参照する必要があるコードのみが参照できるようにする必要があります。キーワード this は現在のクラス インスタンスを参照するため、任意の数のものがそれを参照し、それを使用してスレッド同期を行うことができます。

明確にするために言うと、コードの他のチャンクがクラス インスタンスを使用してロックする可能性があり、コードがタイムリーなロックを取得できなかったり、他のスレッド同期の問題が発生したりする可能性があるため、これは悪いことです。最良のケース: クラスへの参照を使用してロックするものは他にありません。中間ケース: 何かがクラスへの参照を使用してロックを行い、パフォーマンスの問題を引き起こします。最悪の場合: 何かがクラスの参照を使用してロックを行い、非常に悪い、非常に微妙な、非常にデバッグが困難な問題を引き起こします。

于 2008-10-30T19:30:40.570 に答える
-1

同じオブジェクトインスタンスを使用している可能性のある他のリクエストが存在する可能性があるため、インスタンスにパブリックにアクセスできる場合は問題が発生します。プライベート/静的変数を使用することをお勧めします。

于 2012-10-31T22:01:42.143 に答える