2011-01-11 8 views
1

私は、オブジェクトのポインタを格納する順序のないマップを持っています。スレッドの安全性を維持するために正しいことをしているかどうかはわかりません。STLコンテナによるマルチスレッド

typedef std::unordered_map<string, classA*>MAP1; 
MAP1 map1; 
pthread_mutex_lock(&mutexA) 
    if(map1.find(id) != map1.end()) 
    { 
     pthread_mutex_unlock(&mutexA); //already exist, not adding items 
    } 
    else 
    { 
     classA* obj1 = new classA; 
     map1[id] = obj1; 
     obj1->obtainMutex(); //Should I create a mutex for each object so that I could obtain mutex when I am going to update fields for obj1? 
     pthread_mutex_unlock(&mutexA); //release mutex for unordered_map so that other threads could access other object 
     obj1->field1 = 1; 
     performOperation(obj1); //takes some time 
     obj1->releaseMutex(); //release mutex after updating obj1 
    } 
+2

' - > obtainMutex()'が作成される前に別のスレッドがobj1に到達することができない限り、デッドロックが発生する領域はありません。言い換えれば、読み書きのために 'map1'にアクセスする前に' mutexA'をロックしている限り、あなたはOKです。しかし、 'pthread_mutex_unlock(&mutexA)'をマップ内のオブジェクトの内容に基づいてマップのidが変更されない限り、1行上に移動すると思います。 – milkypostman

+0

'map1'への更新のために、複数のスレッドを介して' map'に格納されたオブジェクトがアクセスされていますか?オブジェクトをスレッドセーフなものにするほうが良いのではないですか?その言葉を残念に思っています---あなたのコードは 'obj1-> obtainMutex'ですか? – Jaywalker

+0

私は数年前にMSVCで作業していましたが、標準ライブラリのマルチスレッド版がありました。スレッドセーフなバージョンのstlを手に入れることができれば、コンテナ単位のmutex mutexA – davka

答えて

2

いくつかの考え。

ストアドオブジェクトごとに1つのミューテックスがある場合は、ストアドオブジェクトのコンストラクタにそのミューテックスを作成するようにしてください。言い換えれば、カプセル化を維持するために、外部コードがそのミューテックスを操作するのを避けるべきです。私は内部的にミューテックスを扱うセッター "SetField1"に "field1"を変換します。

次に、私はあなたが最後にobj1->obtainMutex();

前に発生しpthread_mutex_unlock(&mutexA);を動かすことができ、コメントに同意、私はあなたがすべてでobtainMutexを必要としないと思います。あなたのコードは、1つのスレッドだけがオブジェクトの作成を許可されるように見えるので、オブジェクト作成中に1つのスレッドだけが内容を操作します。だから私がここに示したわずかなコードだけを考えれば、オブジェクトあたりのmutexはまったく必要ないとは思われません。

2

私がコードで見た1つの問題は、特に例外が発生したときに問題につながることです。

obj1->obtainMutex(); //Should I create a mutex for each object so that I could obtain mutex when I am going to update fields for obj1? 
    pthread_mutex_unlock(&mutexA); //release mutex for unordered_map so that other threads could access other object 
    obj1->field1 = 1; 
    performOperation(obj1); 

performOperationが例外をスローすると、obj1-> releaseMutex();例外がスローされます。オブジェクトがロックされたままになり、将来的にはデッドロックにつながる可能性があります。 あなたが例外を使用しない場合でも、performOperationで使用するライブラリコードがあります。または、あなたが将来間違って将来間に合って、戻り値を挿入して、以前に所有していたすべてのロックを解除することを忘れるかもしれません...

pthread_mutex_lockとpthread_mutex_unlockの呼び出しは同じです。

ロック/ロック解除にRAIIを使用することをおすすめします。

I.e.コードは次のようになります。

typedef std::unordered_map<string, classA*>MAP1; 

    MAP1 map1; 

    Lock mapLock(&mutexA); //automatci variable. The destructor of the Lock class 
    //automatically calls pthread_mutex_unlock in its destructor if it "owns" the 
    //mutex 

    if(map1.find(id) == map1.end()) 
    { 
     classA* obj1 = new classA; 
     map1[id] = obj1; 

     Lock objLock(obj); 
     mapLock.release(); //we explicitly release mapLock here 

     obj1->field1 = 1; 
     performOperation(obj1); //takes some time 
    } 

Ie.いくつかの最小限のRAAIスレッディングサポートのリファレンスについては、Andrei Alexandrescu(see here)の「Modern C++ design:generic programming and design patterns applied」を参照してください。他のリソースもあります(here

最後に、コードでわかるもう1つの問題について説明します。より正確には、obtainMutexとreleaseMutexをメソッドとして持ち、それらを明示的に呼び出すことで見られる問題です。スレッド1がマップをロックし、オブジェクトを作成してobtainMutexを呼び出し、マップをロック解除するとします。別のスレッド(スレッド2と呼ぶことができます)は実行ロックがスケジュールされます。マップはイテレータをオブジェクトのmap1 [id]に取得し、pObjectのreleaseMutex()を呼び出します(つまりコードが試みないバグobtainMutexを最初に呼び出す)。スレッド1はスケジュールされ、ある時点でreleaseMutex()も呼び出されます。オブジェクトは一度ロックされていますが、二度リリースされました。私が言っていることは、オブジェクトロックインターフェイスのロックを解除しない誤った使用が起こっている可能性があるため、例外が発生したときにコールが常に正しくペア設定されていることを確認する作業が難しくなるということです。また、スレッド2は、マップから取得したpObjectを削除し、マップから消去することもできます。スレッド1は、すでに削除されたオブジェクトを持つ作業に進みます。

RAIIを使用すると、コードをわかりやすく(私たちのバージョンを比較するとさらに短くなります)、上で列挙したいくつかの問題に多く役立ちます。

+1

'if'の中で' Lock mapLock(&mutexA); 'を動かせると思いますか? – davka

+0

@ダフカ。私はそうは思わない。なぜならifがマップにアクセスするから(findを作る)。 map1が複数のスレッド間で共有されているオブジェクトであり、スレッドがmap1.find(id)にある間に別のスレッドがマップに挿入しようとしていると仮定すると、マップへのアクセスを保護する必要があります。ロックを含むスコープを作成して、mapLockが確実に解放されるようにするには、次のようにします。または、ifの後で明示的に解放することができます。私は遅れてマップアクセスを行う関数がifの後に終わると仮定しています.-) – ds27680

+0

もちろん正しいです。私は自分の答えでこのような何かを提案しました:)まあ、私はマルチスレッドで少し錆びたと言いました... – davka

0

答えに私のコメントを組み合わせる思想:

1)あなたは、エントリを追加しているので、コンテナを変更している場合は、コンテナによっては、あなたが、他のスレッドからの読み取りアクセスを許可するべきではありません法的状態間の移行においてコンプリメンタリでは、他のスレッドがそれを読んでいるときにコンテナを変更すべきではありません。これにはread-write lockが必要です。擬似コードのようなものです:私が働いていたとき

set read lock 
search container 
if found 
    release read lock 
    operate on the found object 
else 
    set write lock 
    release read lock 
    add entry 
    release write lock 
endif 

(私がマルチスレッドプログラミングを行ってきたので、私は詳細に錆びたかもしれないので、いくつかの時間がかかった)

2) MSVCは数年前にマルチスレッドの(すなわちスレッドセーフ)バージョンの標準ライブラリを使用しました。それはあなたにこのすべてのトラブルを救うことができます。 gcc/Linuxでもスレッドセーフなstdが存在するかどうかを確認するのは面倒ではありませんでした。

関連する問題