Correct usage of ReentrantReadWriteLock while signalling from writer to reader?

Question

The usage pattern arose from following reasons:

1. I need read thread to wait for data if it is absent with conditions.

2. Read lock does not support condition, so condition should be taken from write lock.

3. Since read thread will wait for condition, it should aquire write lock too to wait.

I have the following locks definition in class:

private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
protected final Lock readLock = rwl.readLock();
protected final Lock writeLock = rwl.writeLock();
protected final Condition hasData = writeLock.newCondition();

In my writer method I have the following pattern:

try {
   writeLock.lock();    

   //...

   if( something_written ) {
      hasData.signalAll();
   }

}
finally {
   writeLock.unlock();
}

In my read method I have the following pattern

try {
   readLock.lock();    

   while( data_absent ) {

      // I need to acquire write lock to wait for condition!
      try {

          // first releasing read lock since we can't acquire write lock otherwise
          // unfortunately this won't release a lock if it was acquired more than once (reentrant)
          readLock.unlock();

          // acquiring write lock to wait it's condition
          writeLock.lock();
          hasData.await(1000, TimeUnit.MILLISECONDS);
      }
      finally {

          // releasing write lock back
          writeLock.unlock();

          // reacquiring read lock
          // again see note about reentrancy
          readLock.lock();
      }


   }

   // reading

}
finally {
   readLock.unlock();
}

Is the pattern above correct?

The problem is that if reader is reentrant, i.e. locking read more than once, then the releasing code does not work and reader hangs at the line of obtaining write lock.

Answer 1

What about the following approach (comments are in code):

public class ReadWrite
{
    private final Lock readLock;
    private final Lock writeLock;
    private final Condition condition;

    {
        ReadWriteLock rwl = new ReentrantReadWriteLock ();
        readLock = rwl.readLock ();
        writeLock = rwl.writeLock ();
        condition = writeLock.newCondition ();
    }

    private Object data;

    // If data is there, return it, otherwise, return null
    private Object tryGetData ()
    {
        readLock.lock ();
        try
        {
            return data; // May return null
        }
        finally
        {
            readLock.unlock ();
        }
    }

    // Wait for data if necessary and then return it
    private Object doGetData () throws InterruptedException
    {
        writeLock.lock ();
        try
        {
            while (data == null)
                condition.await ();

            return data;
        }
        finally
        {
            writeLock.unlock ();
        }
    }

    // Used by reader, return value cannot be null, may block
    public Object getData () throws InterruptedException
    {
        Object result = tryGetData ();
        return result == null ? doGetData () : result;
    }

    // Used by writer, data may be null
    public void setData (Object data)
    {
        writeLock.lock ();
        try
        {
            Object previousData = this.data;
            this.data = data;
            if (previousData == null && data != null)
                condition.notifyAll ();
        }
        finally
        {
            writeLock.unlock ();
        }
    }
}

Answer 2

I think what you are trying to do is make your reader wait for writer to write and then return some value . If there's no value, you want your reader thread to just wait or sleep. Is that correct ? If what I understood is correct, here's one way to do that.

private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
protected final Lock readLock = rwl.readLock();
protected final Lock writeLock = rwl.writeLock();
protected final Condition hasData = writeLock.newCondition();
private HashMap myData = new HashMap(); //example structure to read and write

private final ReentrantLock dataArrivalLock = new ReentrantLock();
private final Condition dataArrivalSignal = dataArrivalLock.newCondition();

Your writer method pattern :

try {
   writeLock.lock();    

   //...
   myData.put("foo","ffoo"); //write something !!
   if( something_written ) {
      hasData.signalAll();
   }

}
finally {
   writeLock.unlock();
}
  try {
                //signal other threads that data has been put in
                dataArrivalLock.lock();
                dataArrivalSignal.signalAll();

            } finally {
                dataArrivalLock.unlock();
            }

Your reader method pattern

try {
            boolean gotData = false;
            while (!gotData) {
                try {
                    readLock.lock();
                    if (myData.size() > 0) {
                        gotData = true;
                        //retrieve the data that is written by writer thred!!
                        myData.get("foo");
                    }
                } finally {
                    readLock.unlock();
                }
                if(!gotData) {
 //sleep the reader thread for x milliseconds. x depends on your application requirement
                  //   Thread.sleep(250);
                    try {
                        //instead of Thread.sleep(), use the dataArrivalLock signal to wakeup
                        dataArrivalLock.lock();
                        dataArrivalSignal.await();
                        //based on how the application works a timed wait might be better !!
                        //dataArrivalSignal.await(250);
                    } finally {
                        dataArrivalLock.unlock();
                    }
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        } 

What this does is force your reader thread to sleep until some data is written by the writer thread .

( Insteadof Thread.sleep(250), you could also use probably an extra lock b/w reader and writer to do the same thing)

Answer 3

Here is what I would do in your situation:

private final ReentrantReadWriteLock    rwl         = new ReentrantReadWriteLock();
protected final Lock                    readLock    = rwl.readLock();
protected final Lock                    writeLock   = rwl.writeLock();
protected final Condition               hasData     = writeLock.newCondition();


public void write() {

    writeLock.lock();
    try {
        // write data
        // ...
        if (something_written) {
            hasData.signalAll();
        }
    }
    finally {
        writeLock.unlock();
    }
}

// replace Object by something else
public Object read() throws InterruptedException {

    Object data = tryRead();

    while (data == null) {
        waitForData();
        data = tryRead();
    }

    return data;
}

// replace Object by something else
private Object tryRead() {

    readLock.lock();
    try {
        Object data = null;
        // read data
        // ...
        // if there no data available, return null
        return data;
    }
    finally {
        readLock.unlock();
    }
}

private void waitForData() throws InterruptedException {

    writeLock.lock();
    try {
        boolean data_available = // check data
        while (!data_available) {
            hasData.await(1000L, TimeUnit.MILLISECONDS);
            data_available = // check data
        }
    }
    finally {
        writeLock.unlock();
    }
}

This is the same behavior of your typical ReadWriteLock usage case if there is available data for reading. If no data exists, then a reader becomes a "writer" (in the lock sense) and waits until some data is available. The cycle repeats until some available data is returned (or until an interrupt occurs).

---

Since you're using a ReadWriteLock, it means you're expecting a much greater number of reads than writes and so you chose a lock that minimizes contention between reader threads (the readLock).

The method waitForData() turns readers into "writers" because they lock on the writeLock instead, resulting in an increased contention between all threads (readers and writers). However, since writes are assumed to be much rarer than reads, a situation where data keeps toggling fast between "available" and "unavailable" is not expected. In other words, assuming writes are rare:

• If there is no available data for reading, then virtually all readers will typically block in the method waitForData() after some time, and will all be notified at the same time when some new data is written.

• If there is some available data for reading, then all readers will simply read it without creating any contention among the threads when locking the readLock.

Answer 4

It sounds like a classic producer/consumer pattern so I suggest you look at existing data structures for this purpose, like on of the BlockingQueue implementations.

Producer thread(s) put() data on the queue, consumer thread(s) take() data from the queue.

Manual synchronization/locking should always be a last resort.

Answer 5

Your usage pattern is wrong: the reader must only engage the read lock; same for the writer. The semantics are such: many readers may acquire the read lock at once, as long as the write lock is free; writer may acquire the write lock only if no other lock is acquired (either read or write).

In your writer code you try to acquire the read lock while you still hold the write lock; similarly for the reader code.