デッドロックの待機/通知

Question

「追加」メソッドと「取得」メソッドには、最初のスレッドがデータを追加し、2番目のスレッドがデータを取得するいくつかのブロック機構を持つキューがあります。上記のコードで

public synchronized MyObj getData() {    
    synchronized (myLock) { 
     synchronized (this) { 
      if (isEmpty()) {     
       wait(0);      
      } 
     }  


     return getData();   
    } 
} 

public synchronized void addData(MyObj data) { 
    if (!isFull()) { 
     putData(data); 
     synchronized (this) { 
      notify(); 
     } 
    } 
} 

、最初のスレッドがデータを取得しようとすると、キューが空である場合、私は待機を経て、待機中に入れた（0）他のスレッドまで通知（）を経由して待ってからリリースをキューにデータを追加します。

今、私はキューがいっぱいになると、いくつかのいずれかがそれにデータを追加しようとしたときに、別の「ロック」を追加します：結果は、私が期待するものではありません

public synchronized MyObj getData() {    
    synchronized (myLock) { 
     synchronized (this) { 
      if (isEmpty()) {     
       wait(0);      
      } 
     }  

     synchronized (this) { 
      notify(); 
     } 
     return getData();   
    } 
} 

public synchronized void addData(MyObj data) { 
    synchronized (myLock) { 
     synchronized (this) { 
      if (isFull()) { 
       wait(0); 
      } 
     } 
    } 

    synchronized (this) { 
     notify(); 
     } 
     PutData(data); 
} 

、私は私が死んを得ることを推測しますロック原因プロセスが停止しています。

UPDATE

これは私がデータを取得する方法である：

queueSize--; 
startPointer = (startPointer + 1) % mqueueSize; 
data = (String) queue[startPointer]; 

これは私はあなたがjava.util.BlockingQueueで見ていないのはなぜ

queueSize++; 
    endPointer = (endPointer + 1) % mqueueSize; 
    queue[endPointer] = data; 

public synchronized boolean isEmpty() { 
     return queueSize== 0; 
    } 

    public synchronized boolean isFull() { 
     return queueSize== mqueueSize; 
    } 

Answer 1

データを追加する方法です。おそらくそれはあなたの状況で役に立つでしょう。

は特にあなたは、コンストラクタでキューの容量を指定した場合、そのキューはブロックしますjava.util.LinkedBlockingQueue、見てみましょう。

Answer 2

メソッドシグネチャから​​キーワードを削除してください。これは、synchronized(this)ブロックが単に冗長であるため、thisモニター全体のメソッド呼び出しを保持することを意味しています。

EDIT：

...次に待ってmyLockはなくthisに通知を呼び出します。 thisの同期について完全に忘れてください。 （あなたの現在のコードでthisに）待っている間、あなたはmyLockロックを解除していないので、他のスレッドはnotify()に取得することができないからです。

Answer 3

whileでifを交換してください。コレクションが本当に空でない/完全でない場合は、チェックを二重にすることを傷つけることはありません。

本当に2つのロックは必要ありません。単一のロックはほとんど同様に機能し、はるかに簡単になるはずです。

public synchronized T get() 
{ 
    while(isEmpty()) 
     wait(0); 

    notifyAll(); 

    return super.get(); 

} 

public synchronized put(T t) 
{ 

    while(isFull()) 
     wait(0); 

    super.put(t); 

    notifyAll(); 

} 

何か変更すると、すべてのスレッドが起動します。しかし、彼らが彼らの仕事をすることができない場合、彼らはの次のnotifyのためになります。

Answer 4

は、なぜあなたは3​​文を持っているのですか？ wait(0)だけでこれだけ1つを保つ方法及びsynchronized(myLock)から​​をダンプ、thisのロックを解除します。

一部のオブジェクト（この場合はthisを呼び出しています）でwaitを呼び出すと、そのオブジェクトのロックが自動的に解放され、他のスレッドが続行できるようになります。しかし、あなたはに既に電話しているので、myLockで待つことは決してありません。その部分は冗長であり、デッドロックを引き起こします。

このシナリオを検討してください：追加しようとしているスレッドはmyLockのロックを受け取りますが、キューがいっぱいになって待機します。この待ち時間はmyLockのロックを解除しません。他のスレッドはデータを取得したいが、最初のスレッドがmyLockのロックを解除しなかったので、ブロック​​を入力することはできません。

結論：synchronized(myLock)ブロックを削除してください。

Answer 5

既に述べたように、コードにはあまりにも多くの注釈があります。​​また、waitの条件はifの条件でチェックされますが、spurious wakeupsを回避するにはwhileのループでチェックするのが理想的です。以下は、これらを修正するコードの概要です。

// _isEmpty and _getData are private unsynchronized methods 
public MyData get() throws InterruptedException { 
    // wait and notify should be called from a block 
    // synchronized on the same lock object (here myLock)  
    synchronized (myLock) { 
    // the condition should be tested in a while loop 
    // to avoid issues with spurious wakeups 
    while (_isEmpty()) { 
     // releases the lock and wait for a notify to be called 
     myLock.wait(); 
    } 
    // when control reaches here, we know for sure that 
    // the queue is not empty 
    MyData data = _getData(); 
    // try to wake up all waiting threads - maybe some thread 
    // is waiting for the queue not to be full 
    myLock.notifyAll(); 
    } 
} 

// _isFull and _putData are private unsynchronized methods 
public void put(MyData obj) throws InterruptedException { 
    synchronized (myLock) { 
    while (_isFull()) { 
     myLock.wait(); 
    } 
    _putData(obj); 
    myLock.notifyAll(); 
    } 
}