[concurrency-interest] ReentrantWriterPreferenceReadWriteLock -- Deadlock if reader releases lock more times than it aquires lock

Mazda.Hewitt@ubsw.com Mazda.Hewitt@ubsw.com
Mon, 2 Dec 2002 14:29:49 -0000


Hi, 

ReentrantWriterPreferenceReadWriteLock appears to deadlock if a reader releases the lock more times than it aquires it.  In this case the reader is releasing a lock that it doesn't own.  I understand that the number of aquires should exactly match the number of releases and you have to be careful to realease all locks that are aquired.  But as you can see in this example it is easy to accidently release the lock too many times.  It is also easy to assume that this won't matter.

It also throws an ugly nullpointer exception when the release is called.

Perhaps there might be a way of checking to see if the thread releasing the lock owns the lock, before it is allowed to release it.

Below I have the current code, then my changed code and followed by my test program.

Mazda Hewitt


-- Code snipet from ReentrantWriterPreferenceReadWriteLock  Current code---

  protected synchronized Signaller endRead() {
    --activeReaders_;
    Thread t = Thread.currentThread();
    Object c = readers_.get(t);
    if (c != IONE) { // more than one hold; decrement count
      int h = ((Integer)(c)).intValue()-1;                 // <--c is null as thread is not in map
      Integer ih = (h == 1)? IONE : new Integer(h);
      readers_.put(t, ih);
      return null;
    }
    else {
      readers_.remove(t);
    
      if (writeHolds_ > 0) // a write lock is still held by current thread
        return null;
      else if (activeReaders_ == 0 && waitingWriters_ > 0)
        return writerLock_;
      else
        return null;
    }
  }

---------------------------  Proposed code : 

 protected synchronized Signaller endRead() {
    
    Thread t = Thread.currentThread();
    Object c = readers_.get(t);
    if ( c != null){  // check to see if this reader owns a read lock
      --activeReaders_;
      if (c != IONE) { // more than one hold; decrement count
        int h = ((Integer)(c)).intValue()-1;
        Integer ih = (h == 1)? IONE : new Integer(h);
        readers_.put(t, ih);
        
      }
      else {
        readers_.remove(t);
        
        if (writeHolds_ > 0) // a write lock is still held by current thread
          return null;
        else if (activeReaders_ == 0 && waitingWriters_ > 0)
          return writerLock_;
        
      }
    }
    return null;
  }

  protected synchronized Signaller endWrite() {
    if (activeWriter_ == Thread.currentThread()){
      --writeHolds_;
      if (writeHolds_ > 0)   // still being held
        return null;
      else {
        activeWriter_ = null;
        if (waitingReaders_ > 0 && allowReader())
          return readerLock_;
        else if (waitingWriters_ > 0)
          return writerLock_;
        else
          return null;
      }
    }
    return null;
  }

---------------------------

---------------------------------------------------------------
Program output:
done Reader Thread-1 Reading
Reader Thread-3 Failed to get lock 
Writer Thread-4 Writing 
Reader Thread-5 Failed to get lock
Reader Thread-7 Failed to get lock
Reader Thread-9 Failed to get lock 
Reader Thread-11 Failed to get lock 
Reader Thread-13 Failed to get lock 
Reader Thread-15 Failed to get lock 
Reader Thread-17 Failed to get lock 
Reader Thread-19 Failed to get lock

---------------------------------------------------------------
import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock;

public class Tester {
  static ReentrantWriterPreferenceReadWriteLock lock = new ReentrantWriterPreferenceReadWriteLock();
  public static void main(String[] args) {

    for (int i =0; i< 10; i++){
      new Reader().start();
      new Writer().start();
    }

    System.out.println("done");
  }

  static class Reader extends Thread{
    public void run(){
      try {
        if ( lock.readLock().attempt(100)){
          System.out.println("Reader " + getName()+" Reading");
          sleep (2000);
        }
        else{
           System.out.println("Reader " + getName()+" Failed to get lock");
        }
      }
      catch (InterruptedException ex) {
        interrupt();
      }
      finally{
        lock.readLock().release();
      }
    }
  }

  static class Writer extends Thread{
    public void run(){
      try {
        lock.writeLock().acquire();
        System.out.println("Writer " + getName()+" Writing");
        sleep (2000);
      }
      catch (InterruptedException ex) {
        interrupt();
      }
      finally{
        lock.writeLock().release();
      }
    }
  }
}

Visit our website at http://www.ubswarburg.com

This message contains confidential information and is intended only 
for the individual named.  If you are not the named addressee you 
should not disseminate, distribute or copy this e-mail.  Please 
notify the sender immediately by e-mail if you have received this 
e-mail by mistake and delete this e-mail from your system.

E-mail transmission cannot be guaranteed to be secure or error-free 
as information could be intercepted, corrupted, lost, destroyed, 
arrive late or incomplete, or contain viruses.  The sender therefore 
does not accept liability for any errors or omissions in the contents 
of this message which arise as a result of e-mail transmission.  If 
verification is required please request a hard-copy version.  This 
message is provided for informational purposes and should not be 
construed as a solicitation or offer to buy or sell any securities or 
related financial instruments.