2012-01-19 18 views
5

私はlock(this)または共有オブジェクトを使用するのが間違っていることを知っています。このロック使用スレッドは安全ですか?

この使用法は問題ありませんか?

public class A 
{ 
    private readonly object locker = new object(); 
    private List<int> myList; 
    public A() 
    { 
    myList = new List<int>() 
    } 

    private void MethodeA() 
    { 
    lock(locker) 
    { 
     myList.Add(10); 
    } 
    } 

    public void MethodeB() 
    { 
    CallToMethodInOtherClass(myList); 
    } 
} 

public class OtherClass 
{ 
    private readonly object locker = new object(); 
    public CallToMethodInOtherClass(List<int> list) 
    { 
    lock(locker) 
    { 
    int i = list.Count; 
    } 
    } 
} 

このスレッドは安全ですか? OtherClass私たちはプライベートオブジェクトでロックしていますので、class Aプライベートロックでロックすると、リストはロックブロックでまだ変更できますOtherClass

+11

バスルームには2つのドアがあり、それぞれにロックが付いています。あなたの質問は、「私がシャワーを浴びているときに初めてロックをロックすると仮定し、シャワーにいるときに友人のボブだけがロックをロックしています。明らかにはい!あなたとボブが一緒にシャワーを避けたいのであれば、同じロック*を使うことに同意する必要があります。このように、スレッドセーフなオブジェクトにはアクセスできません。 –

答えて

10

いいえ、スレッドセーフではありません。 AddとCountは、同じ時間に実行されます。 2つの異なるロックオブジェクトがあります。リストを渡すときに

常に独自のロックオブジェクトをロック:

public void MethodeB() 
    { 
    lock(locker) 
    { 
     CallToMethodInOtherClass(myList); 
    } 
    } 
+0

実際にはスレッドセーフですが、 'Count'の実装の詳細はスレッドセーフであり、これは約束されておらず、より現実的なコードに拡張されません。 –

2

いいえ、スレッドセーフではありません。

あなたの2つのメソッドは2つの異なるオブジェクトでロックされていますが、互いにロックアウトすることはありません。

CallToMethodInOtherClass()はCountの値のみを取得するため、何もひどく間違っています。しかし、その周りのlock()は役に立たず、誤解を招く。

この方法でリストを変更すると、問題が発生します。それを解決するには、MethodeB:

public void MethodeB() 
    { 
    lock(locker) // same instance as MethodA is using 
    { 
     CallToMethodInOtherClass(myList); 
    } 
    } 
+0

私が思ったこと。どのように正しい方法を教えていただけますか? – Maya

2

を変更します。いいえ、同じオブジェクトをロックする必要があります。あなたのコードでは、それらは両方とも異なるものにロックされ、各呼び出しは同時に実行できます。

コードスレッドを安全にするには、MethodeBをロックするか、またはリスト自体をロックオブジェクトとして使用します。

+0

@ Felix共有オブジェクトでロックするのが間違っているので、リストそのものをロックするのが間違っていますか? – Maya

+0

@マヤそれは間違いだ。しかし、それは説明された問題を解決する簡単な方法です。しかし、とにかく、正しい方法は、スレッドセーフなリストまたはリストを含むコンテナを作成し、コンテナの上だけにリストにアクセスすることです。 –

3

はありません、これはスレッドセーフではありません。 A.MethodeAOtherClass.CallToMethodInOtherClassは異なるオブジェクトでロックされているため、相互に排他的ではありません。リストへのアクセスを保護する必要がある場合は、それを非公開にして外部コードに渡さないでください。すべての答えが、これらは別のロックオブジェクトであると言うトリック

public class A 
{ 
    private List<int> myList; 
    public A() 
    { 
    myList = new List<int>() 
    } 

    private void MethodeA() 
    { 
    lock(myList) 
    { 
     myList.Add(10); 
    } 
    } 

    public void MethodeB() 
    { 
    CallToMethodInOtherClass(myList); 
    } 
} 

public class OtherClass 
{ 
    public CallToMethodInOtherClass(List<int> list) 
    { 
    lock(list) 
    { 
    int i = list.Count; 
    } 
    } 
} 
0

おそらく最も簡単な方法。

簡単な方法は、静的なロックオブジェクトのf.exを持つことです。

lock(A.lockObj) 
{ 
} 
+0

しかし、私はshraedオブジェクトとロックするためにその先を考えた – Maya

1

尻を行うには

3

ありません、これはスレッドセーフではありません。

publc class A 
{ 
    public static readonly object lockObj = new object(); 
} 

と両方のクラスには次のようにロックを使用します。スレッドセーフなものにするには、スレッド間で共有されるため、staticオブジェクトのロックを使用できます。これはコード内にデッドロックを引き起こす可能性がありますが、ロックの正しい順序を維持することで処理できます。 lockに関連するパフォーマンスコストがありますので、それを賢明に使用してください。

希望します。

+0

これはどのように私を助けますか? – Maya

+0

ありがとう、今私は理解している – Maya

+0

静的オブジェクトは他のオブジェクト間で共有されているので、これをロックすると、一度に1つのオブジェクトだけが消費されます。 [静的MSDN](http://msdn.microsoft.com/en-us/library/79b3xss3(v=80).aspx)または[MSDNでロック](http://msdn.microsoft .com/en-us/library/c5kehkcz(v = vs80).aspx)(_ベストプラクティスは、ロックするプライベートオブジェクトを定義するか、またはすべてのインスタンスに共通するデータを保護するプライベートスタティックオブジェクト変数です._ MSDN) –

0

静的な読み取り専用ロックを使用した回答が多数あります。

しかし、この静的ロックは避けてください。複数のスレッドが静的ロックを使用しているデッドロックを作成するのは簡単です。

代わりに、.net 4並行コレクションの1つが使用できます。これは、ロックを使用する必要がないように、スレッド同期を提供します。

System.collections.Concurrent名前空間を参照してください。 この例では、ConcurrentBag<T>クラスを使用できます。

1

それは実際にスレッドセーフですが、(純粋にCount上の実装の詳細の問題として):コードの

  1. スレッドセーフなスニペットがないスレッドセーフなアプリケーション作るのですか。異なるスレッドセーフな操作をスレッドセーフでない操作に組み合わせることができます。実際、多くのスレッドセーフでないコードは、スレッドセーフである小さな部分に分割することができます。

  2. あなたが望んでいた理由からスレッドセーフではありません。つまり、それをさらに拡張するとスレッドセーフではありません。

このコードはスレッドセーフになります:任意のリストと

public void CallToMethodInOtherClass(List<int> list) 
{ 
    //note we've no locks! 
    int i = list.Count; 
    //do something with i but don't touch list again. 
} 

コールそれを、そしてそれは関係なく、他のスレッドが何であるかを、そのリストの状態に基づいて値をiあげますまでlistは壊れません。 iに無効な値が設定されることはありません。だから、

このコードは、スレッドセーフである間:

public void CallToMethodInOtherClass(List<int> list) 
{ 
    Console.WriteLine(list[93]); // obviously only works if there's at least 94 items 
          // but that's nothing to do with thread-safety 
} 

このコードはスレッドセーフではありません。

public void CallToMethodInOtherClass(List<int> list) 
{ 
    lock(locker)//same as in the question, different locker to that used elsewhere. 
    { 
    int i = list.Count; 
    if(i > 93) 
     Console.WriteLine(list[93]); 
    } 
} 

先に進む前に、私はスレッドように説明2ビット安全はリストの仕様であると約束されていません。保守的なコーディングでは、実装の詳細に依存するのではなく、スレッドセーフではないとみなされますが、重要な方法でロックを使用する方法の問題に影響するため、実装の詳細に依存します:

最初にlockerのロックを取得していないlistで動作するコードは、そのコードがCallToMethodInOtherClassと同時に実行されることを妨げられません。さて、list.Countはスレッドセーフでlist[93]はトレッドセーフですが、2番目の動作を確実にするために最初に依存する2つの組み合わせは、スレッドセーフではありません。ロック外のコードはlistに影響する可能性があるため、またはClearCountの間で呼び出し、list[93]が動作し、list[93]が呼び出されることがあります。

listがこれまでに追加されたことがわかっている場合は、サイズ変更が同時に発生しても、いずれかの方法でlist[93]という値になります。何かがlist[93]に書き込んでいて、.NETがアトミックに書き込むタイプ(そしてintがそのようなタイプの1つです)の場合、正しくロックしたかのように、古いものか新しいもののどちらかになりますどのスレッドが最初にロックに行くかによって古いものか新しいものかを取得します。また、これは指定された約束の実装の詳細ではありませんが、スレッド安全性がまだスレッドセーフなコードではないことを指摘しています。

これを実際のコードに移動します。 list.Countlist[93]はスレッドセーフであると仮定すべきではありません。なぜなら、私たちは、それが変更される可能性があると約束されていないためですが、その約束があったとしても、それらの約束は合致しませんスレッドセーフです。

重要なことは、相互に干渉する可能性のあるブロックのコードを保護するために同じロックを使用することです。したがって、その下のバリアントはスレッドセーフであることが保証されて考えてみましょう。

public class ThreadSafeList 
{ 
    private readonly object locker = new object(); 
    private List<int> myList = new List<int>(); 

    public void Add(int item) 
    { 
    lock(locker) 
     myList.Add(item); 
    } 
    public void Clear() 
    { 
    lock(locker) 
     myList.Clear(); 
    } 
    public int Count 
    { 
    lock(locker) 
     return myList.Count; 
    } 
    public int Item(int index) 
    { 
    lock(locker) 
     return myList[index]; 
    } 
} 

このクラスは、それが行うすべてにスレッドセーフであることが保証されます。実装の詳細に依存せずに、同じインスタンスで別のスレッドが行っているために、状態が破損したり不正確な結果が返されるメソッドはありません。次のコードはまだはしかし動作しません:

// (l is a ThreadSafeList visible to multiple threads. 
if(l.Count > 0) 
    Console.WriteLine(l[0]); 

を私たちは、各呼び出し100%のスレッドの安全性を保証してきましたが、我々は組み合わせを保証していない、と私たち保証することはできません組み合わせ。

私たちができることは2つあります。その組み合わせのためのメソッドを追加することができます。次のようなものは、特にマルチスレッドの使用のために設計された多くのクラスのための共通のようになります。

public bool TryGetItem(int index, out int value) 
{ 
    lock(locker) 
    { 
    if(l.Count > index) 
    { 
     value = l[index]; 
     return true; 
    } 
    value = 0; 
    return false; 
    } 
} 

これは、カウントテストとスレッドセーフであることが保証された単一の操作の項目の検索部分になります。

また

、最も頻繁に私達は何をする必要があるか、我々はロック操作がグループ化されている場所で起こるがあります。もちろん

lock(lockerOnL)//used by every other piece of code operating on l 
    if(l.Count > 0) 
    Console.WriteLine(l[0]); 

、これは冗長ThreadSafeListとちょうど廃棄物の中にロックを作ります努力、宇宙、そして時間のこれは、ほとんどのクラスがインスタンスメンバーにスレッドセーフティを提供しない主な理由です。クラス内のメンバーのコールグループを意味のある方法で保護できないため、スレッドセーフが約束していない限り、彼ら自身で非常によく指定され、有用である。戻ってあなたの問題のコードに来て

OtherClassが内部でロックするための独自の理由がない限り、CallToMethodInOtherClass

ロックが除去されなければなりません。それは非スレッドセーフな方法で結合されず、プログラムにロックを追加するだけで、デッドロックがないことを確かめるためにそれを分析する複雑さが増すという意味のある約束をすることはできません。

CallToMethodInOtherClassへの呼び出しは、そのクラス内の他の操作と同じロックによって保護されるべきである。

public void MethodeB() 
{ 
    lock(locker) 
    CallToMethodInOtherClass(myList); 
} 

その後限りCallToMethodInOtherClassが、それは後に他のスレッドが見ることができるどこかmyListが格納されていないとして、 myListにアクセスできる唯一のコードは、myListの他の操作と並行して呼び出さないことを保証するため、CallToMethodInOtherClassはスレッドセーフではありません。

二つの重要な事柄があります:

  1. 何かを「スレッドセーフ」と記載されている場合は、スレッドセーフ」に該当約束の異なる種類があるので、それは、それによって有望だだけで何を知っています「それを意味するものではありませんが、私はこの物体を無意味な状態に置かないでしょう」ということを意味しています。おそらくこれでボールをプレーしていない他のスレッドがないことができるように、同じデータに影響を与え、そしてオブジェクトへのアクセスを守るよ、グループごとに同じロック付きグループ事業の

  2. ロック、。

*これはスレッドセーフの定義が非常に限定されています。 List<T>list[93]を呼び出します。Tは、原子的に書き込まれ、読み取られるタイプで、実際に少なくとも94個のアイテムがあるかどうかわかりません。他のスレッドが動作しているかどうかにかかわらず、等しく安全です。もちろん、どちらの場合でもArgumentOutOfRangeExceptionを投げることができるということは、ほとんどの人が「安全」と考えるようなものではありませんが、複数のスレッドでの保証は同じです。単一のスレッドでCountをチェックすることでより強力な保証を得ることができますが、マルチスレッドの状況ではスレッドセーフではないと記述することができません。そのコンボはまだ状態を破壊しませんが、私たち自身が起こることができないことを保証した例外につながる可能性があります。

+0

代わりにConcurrentBagまたはConcurrentDictionaryを使用すると、ロックを心配する必要はありません。 –

+0

@PålThingbø少しでも真実ではありません。まず、これらのコレクション、実際には自分の[スレッドセーフな辞書とセット](https://bitbucket.org/JonHanna/ariadne)と他のコレクションはスレッドセーフです。それらのアクション、アクションのグループ上記の理由からスレッドセーフであるとは限りません( 'int'はスレッドセーフですが、' ++ x'はスレッドセーフではありません。エンティティはユニットとしてスレッドセーフではありません)。リストを望むときに、バッグや辞書を使用することを提案することはほとんどありません。 ... –

+0

@PålThingbøこれらのクラスのセマンティクスはまったく異なります。私はうまくいけばスレッドセーフなリストクラスをリリースしていますが、これは上記の答えよりもはるかに優れた並行動作を提供しますが、まだ制限されています( 'RemoveAt'や' Insert'はありません;あまりにも遅いですが、ロックなしでスレッドセーフであることは、他のものよりもはるかに面倒です)、そしてそれでも 'ConcurrentDictionary'よりも魔法のスレッドセーフであるコードを作成することはありません私は上記を与える。 –

関連する問題