Concurrent read/write buffer in java

Question

I am trying to implement a read/write buffer class where it can be able to support multiple writers and readers, and the reader can read the buffer simultaneously while the writer is writing the buffer. Here's my code, and so far I haven't seen any issue, but I am not 100% sure if this is thread-safe or if there's any better approach.

public class Buffer{
       private StringBuilder sb = new StringBuilder();
       private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
       private Random random = new Random();

       public void read(){
            try{
                lock.readLock().lock();
                System.out.println(sb.toString());
            } finally{
                lock.readLock().unlock();
            }
       }
       public void write(){
            try{
                lock.writeLock().lock();
                sb.append((char)(random.nextInt(26)+'a'));
            } finally{
                lock.writeLock().unlock();
            }
       }
} 

Answer 1

No problem with multi-threading safety whatsoever! The read and write locks protect access to the StringBuilder and the code is clean and easy to read.

By using ReentrantReadWriteLock you are actually maximising you chance of achieving higher degrees of concurrency, because multiple readers can proceed together, so this is a better solution than using plain old synchronised methods. However, contrary to what is stated in the question, the code does not allow a writer to write while the readers are reading. This is not necessarily a problem in itself though.

The readers acquire a read lock before proceeding. The writers acquire a write lock before proceeding. The rules of read locks allow one to be acquired when there is no write lock (but it is OK if there are some read locks i.e. if there are more active readers). The rules of write locks allow one to be acquired if and only if there are no other locks (no readers, no writers). Thus multiple readers are allowed but only a single writer.

The only change that might be needed would be to change the lock initialisation code to this:

private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);

As is the original code given in the question does not require the lock to be fair. With the above change it is guaranteed that "threads contend for entry using an approximately arrival-order policy. When the write lock is released either the longest-waiting single writer will be assigned the write lock, or if there is a reader waiting longer than any writer, the set of readers will be assigned the read lock. When constructed as non-fair, the order of entry to the lock need not be in arrival order." (Taken from http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html)

See also the following (from the same source):

ReentrantReadWriteLocks can be used to improve concurrency in some uses of some kinds of Collections. This is typically worthwhile only when the collections are expected to be large, accessed by more reader threads than writer threads, and entail operations with overhead that outweighs synchronization overhead. For example, here is a class using a TreeMap that is expected to be large and concurrently accessed.

class RWDictionary {
    private final Map<String, Data>  m = new TreeMap<String, Data>();
    private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
    private final Lock r = rwl.readLock();
    private final Lock w = rwl.writeLock();

    public Data get(String key) {
        r.lock(); try { return m.get(key); } finally { r.unlock(); }
    }
    public String[] allKeys() {
       r.lock(); try { return m.keySet().toArray(); } finally { r.unlock(); }
    }
    public Data put(String key, Data value) {
        w.lock(); try { return m.put(key, value); } finally { w.unlock(); }
    }
    public void clear() {
        w.lock(); try { m.clear(); } finally { w.unlock(); }
   }
 }

The excerpt from the API documentation is particularly performance conscious. In your specific case, I cannot comment on whether you meet the "large collection" criterion, but I can say that outputting to the console is much more time consuming than the thread-safety mechanism overhead. At any rate, you use of ReentrantReadWriteLocks makes perfect sense from a logical point of view and is perfectly thread safe. This is nice code to read :-)

Note 1 (answering question about exceptions found in the comments of the original question): Taken from http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/Lock.html lock() acquires the lock. If the lock is not available then the current thread becomes disabled for thread scheduling purposes and lies dormant until the lock has been acquired.

A Lock implementation may be able to detect erroneous use of the lock, such as an invocation that would cause deadlock, and may throw an (unchecked) exception in such circumstances. The circumstances and the exception type must be documented by that Lock implementation.

No indication of such exceptions is given in the relevant documentation for ReentrantReadWriteLock.ReadLock (http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.ReadLock.html) or ReentrantReadWriteLock.WriteLock (http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.WriteLock.html)

Note 2: While access to the StringBuilder is protected by the locks, System.out is not. In particular, multiple readers may read the value concurrently and try to output it concurrently. That is also OK, because access to System.out.println() is synchronized.

Note 3: If you want to disallow multiple active writers, but allow a writer and one or more readers to be active at the same time, you can simple skip using read locks altogether i.e. delete lock.readLock().lock(); and lock.readLock().unlock(); in your code. However, in this particular case this would be wrong. You need to stop concurrent reading and writing to the StringBuilder.