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

David Holmes dholmes@dltech.com.au
Tue, 3 Dec 2002 07:01:00 +1000


Mazda Hewitt wrote:
> 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.

Thanks for the posting.

While I'm all for making misuse of these things more difficult, and
making the consequence of misuse less drastic (assuming the price of
that is not too high), I must emphasise the importance of the correct
usage of the try/finally construct - it is something that a number of
books and articles often get wrong.

[The following is not a lecture to the original poster but rather a
clarification for anyone reading the list archive at a later date.]

In the example, we have:

    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();
      }
    }

which is an incorrect usage of the try/finally construct as the
finally clause is executed even if the Lock.attempt failed. With
paired actions like acquire/attempt and release, it is crucial to
remember that the first action occurs *outside* of the try block:

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

Now sometimes the structure of code is such that doing try/finally
correctly, and dealing with exceptions, can lead to awkward structure.
In such cases you will often find the code in the finally clause
protected by a condition - such as checking a reference to an I/O
stream for null before closing it.

<end lecture> :)

David Holmes