75

I am using in my code at the moment a ReentrantReadWriteLock to synchronize access over a tree-like structure. This structure is large, and read by many threads at once with occasional modifications to small parts of it - so it seems to fit the read-write idiom well. I understand that with this particular class, one cannot elevate a read lock to a write lock, so as per the Javadocs one must release the read lock before obtaining the write lock. I've used this pattern successfully in non-reentrant contexts before.

What I'm finding however is that I cannot reliably acquire the write lock without blocking forever. Since the read lock is reentrant and I am actually using it as such, the simple code

lock.getReadLock().unlock();
lock.getWriteLock().lock()

can block if I have acquired the readlock reentrantly. Each call to unlock just reduces the hold count, and the lock is only actually released when the hold count hits zero.

EDIT to clarify this, as I don't think I explained it too well initially - I am aware that there is no built-in lock escalation in this class, and that I have to simply release the read lock and obtain the write lock. My problem is/was that regardless of what other threads are doing, calling getReadLock().unlock() may not actually release this thread's hold on the lock if it acquired it reentrantly, in which case the call to getWriteLock().lock() will block forever as this thread still has a hold on the read lock and thus blocks itself.

For example, this code snippet will never reach the println statement, even when run singlethreaded with no other threads accessing the lock:

final ReadWriteLock lock = new ReentrantReadWriteLock();
lock.getReadLock().lock();

// In real code we would go call other methods that end up calling back and
// thus locking again
lock.getReadLock().lock();

// Now we do some stuff and realise we need to write so try to escalate the
// lock as per the Javadocs and the above description
lock.getReadLock().unlock(); // Does not actually release the lock
lock.getWriteLock().lock();  // Blocks as some thread (this one!) holds read lock

System.out.println("Will never get here");

So I ask, is there a nice idiom to handle this situation? Specifically, when a thread that holds a read lock (possibly reentrantly) discovers that it needs to do some writing, and thus wants to "suspend" its own read lock in order to pick up the write lock (blocking as required on other threads to release their holds on the read lock), and then "pick up" its hold on the read lock in the same state afterwards?

Since this ReadWriteLock implementation was specifically designed to be reentrant, surely there is some sensible way to elevate a read lock to a write lock when the locks may be acquired reentrantly? This is the critical part that means the naive approach does not work.

AlikElzin-kilaka
  • 34,335
  • 35
  • 194
  • 277
Andrzej Doyle
  • 102,507
  • 33
  • 189
  • 228
  • Does your problem come from trying to do an unknowable number of read locks on the same thread you're doing the writing. Couldn't you do the writing on another thread? – justinhj Jun 03 '09 at 17:34

12 Answers12

32

This is an old question, but here's both a solution to the problem, and some background information.

As others have pointed out, a classic readers-writer lock (like the JDK ReentrantReadWriteLock) inherently does not support upgrading a read lock to a write lock, because doing so is susceptible to deadlock.

If you need to safely acquire a write lock without first releasing a read lock, there is a however a better alternative: take a look at a read-write-update lock instead.

I've written a ReentrantReadWrite_Update_Lock, and released it as open source under an Apache 2.0 license here. I also posted details of the approach to the JSR166 concurrency-interest mailing list, and the approach survived some back and forth scrutiny by members on that list.

The approach is pretty simple, and as I mentioned on concurrency-interest, the idea is not entirely new as it was discussed on the Linux kernel mailing list at least as far back as the year 2000. Also the .Net platform's ReaderWriterLockSlim supports lock upgrade also. So effectively this concept had simply not been implemented on Java (AFAICT) until now.

The idea is to provide an update lock in addition to the read lock and the write lock. An update lock is an intermediate type of lock between a read lock and a write lock. Like the write lock, only one thread can acquire an update lock at a time. But like a read lock, it allows read access to the thread which holds it, and concurrently to other threads which hold regular read locks. The key feature is that the update lock can be upgraded from its read-only status, to a write lock, and this is not susceptible to deadlock because only one thread can hold an update lock and be in a position to upgrade at a time.

This supports lock upgrade, and furthermore it is more efficient than a conventional readers-writer lock in applications with read-before-write access patterns, because it blocks reading threads for shorter periods of time.

Example usage is provided on the site. The library has 100% test coverage and is in Maven central.

npgall
  • 2,979
  • 1
  • 24
  • 24
  • Is this still maintained? I see that the readme has been updated but the code hasn't changed for 2 years. – Navin Sep 20 '15 at 12:31
  • 4
    I'm the author and I resent the accusation that a project which still works requires constant attention :) The project is still maintained, but unless someone logs a bug or feature request, there is nothing more to be done. The library has full test coverage and development is complete. – npgall Sep 21 '15 at 21:21
  • Ah ok, it's just rare to see a "complete" OSS project. Is there any chance this will be in the Java n+1 standard lib? It seems like the kind of thing that a lot of people reinvent. – Navin Sep 22 '15 at 09:21
  • Yes I agree. I asked on the concurrency-interest list if it was something that could eventually be included in the JDK. But the discussion focussed on other points and nobody gave an opinion on that one. I'd like to see it included personally as well. – npgall Sep 22 '15 at 14:15
  • This is awesome. This answer should have received far more votes than 12. Well, 13 now. – Mike Nakis Mar 23 '17 at 18:57
29

I have made a little progress on this. By declaring the lock variable explicitly as a ReentrantReadWriteLock instead of simply a ReadWriteLock (less than ideal, but probably a necessary evil in this case) I can call the getReadHoldCount() method. This lets me obtain the number of holds for the current thread, and thus I can release the readlock this many times (and reacquire it the same number afterwards). So this works, as shown by a quick-and-dirty test:

final int holdCount = lock.getReadHoldCount();
for (int i = 0; i < holdCount; i++) {
   lock.readLock().unlock();
}
lock.writeLock().lock();
try {
   // Perform modifications
} finally {
   // Downgrade by reacquiring read lock before releasing write lock
   for (int i = 0; i < holdCount; i++) {
      lock.readLock().lock();
   }
   lock.writeLock().unlock();
}

Still, is this going to be the best I can do? It doesn't feel very elegant, and I'm still hoping that there's a way to handle this in a less "manual" fashion.

Skcussm
  • 658
  • 1
  • 5
  • 20
Andrzej Doyle
  • 102,507
  • 33
  • 189
  • 228
  • 1
    Furthermore, I think getReadHoldCount returns the count for all threads (although the doc doesn't say it clearly). While RRWL permits releasing other threads' read locks, your second for loop is reacquiring them all with the current thread, which is more than you want. – Olivier Jan 21 '09 at 13:17
  • 16
    I think you're looking at getRead*LOCK*Count*(), rather than getRead*HOLD*Count*(). The latter has no such warning in the Javadocs and does return the number of holds for the current thread (e.g. lock twice in thread A, then 3 times in thread B, it will return 2 in thread A and not 5). – Andrzej Doyle Jan 21 '09 at 14:16
  • 1
    Exact! That pretty much voids my two comments :-) – Olivier Jan 22 '09 at 10:52
  • 1
    Cool solution. I will slip this into my bag-o-tricks for later. – Mike Clark Nov 05 '10 at 03:14
  • 3
    You can lose the consistency of tree. Read lock is made to be sure that nothing will changed while working with it. And at the same time you try to change this structure with our own hands. So when you continue iterating over tree - it can be absolutely another and you will have to return to the beginning of iteration. – porfirion Apr 07 '17 at 17:13
25

What you want to do ought to be possible. The problem is that Java does not provide an implementation that can upgrade read locks to write locks. Specifically, the javadoc ReentrantReadWriteLock says it does not allow an upgrade from read lock to write lock.

In any case, Jakob Jenkov describes how to implement it. See http://tutorials.jenkov.com/java-concurrency/read-write-locks.html#upgrade for details.

Why Upgrading Read to Write Locks Is Needed

An upgrade from read to write lock is valid (despite the assertions to the contrary in other answers). A deadlock can occur, and so part of the implementation is code to recognize deadlocks and break them by throwing an exception in a thread to break the deadlock. That means that as part of your transaction, you must handle the DeadlockException, e.g., by doing the work over again. A typical pattern is:

boolean repeat;
do {
  repeat = false;
  try {
   readSomeStuff();
   writeSomeStuff();
   maybeReadSomeMoreStuff();
  } catch (DeadlockException) {
   repeat = true;
  }
} while (repeat);

Without this ability, the only way to implement a serializable transaction that reads a bunch of data consistently and then writes something based on what was read is to anticipate that writing will be necessary before you begin, and therefore obtain WRITE locks on all data that are read before writing what needs to be written. This is a KLUDGE that Oracle uses (SELECT FOR UPDATE ...). Furthermore, it actually reduces concurrency because nobody else can read or write any of the data while the transaction is running!

In particular, releasing the read lock before obtaining the write lock will produce inconsistent results. Consider:

int x = someMethod();
y.writeLock().lock();
y.setValue(x);
y.writeLock().unlock();

You have to know whether someMethod(), or any method it calls, creates a reentrant read lock on y! Suppose you know it does. Then if you release the read lock first:

int x = someMethod();
y.readLock().unlock();
// problem here!
y.writeLock().lock();
y.setValue(x);
y.writeLock().unlock();

another thread may change y after you release its read lock, and before you obtain the write lock on it. So y's value will not be equal to x.

Test Code: Upgrading a read lock to a write lock blocks:

import java.util.*;
import java.util.concurrent.locks.*;

public class UpgradeTest {

    public static void main(String[] args) 
    {   
        System.out.println("read to write test");
        ReadWriteLock lock = new ReentrantReadWriteLock();

        lock.readLock().lock(); // get our own read lock
        lock.writeLock().lock(); // upgrade to write lock
        System.out.println("passed");
    }

}

Output using Java 1.6:

read to write test
<blocks indefinitely>
Bob
  • 251
  • 3
  • 3
12

What you are trying to do is simply not possible this way.

You cannot have a read/write lock that you can upgrade from read to write without problems. Example:

void test() {
    lock.readLock().lock();
    ...
    if ( ... ) {
        lock.writeLock.lock();
        ...
        lock.writeLock.unlock();
    }
    lock.readLock().unlock();
}

Now suppose, two threads would enter that function. (And you are assuming concurrency, right? Otherwise you would not care about locks in the first place....)

Assume both threads would start at the same time and run equally fast. That would mean, both would acquire a read lock, which is perfectly legal. However, then both would eventually try to acquire the write lock, which NONE of them will ever get: The respective other threads hold a read lock!

Locks that allow upgrading of read locks to write locks are prone to deadlocks by definition. Sorry, but you need to modify your approach.

Steffen Heil
  • 4,286
  • 3
  • 32
  • 35
6

Java 8 now has a java.util.concurrent.locks.StampedLock with a tryConvertToWriteLock(long) API

More info at http://www.javaspecialists.eu/archive/Issue215.html

qwerty
  • 3,801
  • 2
  • 28
  • 43
4

What you're looking for is a lock upgrade, and is not possible (at least not atomically) using the standard java.concurrent ReentrantReadWriteLock. Your best shot is unlock/lock, and then check that noone made modifications inbetween.

What you're attempting to do, forcing all read locks out of the way is not a very good idea. Read locks are there for a reason, that you shouldn't write. :)

EDIT:
As Ran Biron pointed out, if your problem is starvation (read locks are being set and released all the time, never dropping to zero) you could try using fair queueing. But your question didn't sound like this was your problem?

EDIT 2:
I now see your problem, you've actually acquired multiple read-locks on the stack, and you'd like to convert them to a write-lock (upgrade). This is in fact impossible with the JDK-implementation, as it doesn't keep track of the owners of the read-lock. There could be others holding read-locks that you wouldn't see, and it has no idea how many of the read-locks belong to your thread, not to mention your current call-stack (i.e. your loop is killing all read locks, not just your own, so your write lock won't wait for any concurrent readers to finish, and you'll end up with a mess on your hands)

I've actually had a similar problem, and I ended up writing my own lock keeping track of who's got what read-locks and upgrading these to write-locks. Although this was also a Copy-on-Write kind of read/write lock (allowing one writer along the readers), so it was a little different still.

falstro
  • 34,597
  • 9
  • 72
  • 86
  • Right, that's the issue. I'm not trying to force ALL read locks out of the way, just the ones held by the current thread. I'm happy for the write lock acquisition to wait for *other* threads to release their read locks. – Andrzej Doyle Jan 21 '09 at 11:38
  • In fact the JDK implementation does keep track of who locked the readlock to some extent; getReadHoldCount() returns the count for the current thread only (via a ThreadLocal) so exactly the right number of unlocks() can be called in this situation. The code I put in my own answer does in fact work. – Andrzej Doyle Jan 21 '09 at 11:50
  • interesting, then this has been changed in jdk1.6, jdk1.5 does not use thread locals at all. – falstro Jan 21 '09 at 11:53
  • 1
    There is no way to upgrade a read lock to a write lock that is guaranteed to succeed. A variant of "unlock/lock, and then check that no one made modifications in between" worked for me. – Seun Osewa Jan 12 '10 at 23:27
2

What about this something like this?

class CachedData
{
    Object data;
    volatile boolean cacheValid;

    private class MyRWLock
    {
        private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
        public synchronized void getReadLock()         { rwl.readLock().lock(); }
        public synchronized void upgradeToWriteLock()  { rwl.readLock().unlock();  rwl.writeLock().lock(); }
        public synchronized void downgradeToReadLock() { rwl.writeLock().unlock(); rwl.readLock().lock();  }
        public synchronized void dropReadLock()        { rwl.readLock().unlock(); }
    }
    private MyRWLock myRWLock = new MyRWLock();

    void processCachedData()
    {
        myRWLock.getReadLock();
        try
        {
            if (!cacheValid)
            {
                myRWLock.upgradeToWriteLock();
                try
                {
                    // Recheck state because another thread might have acquired write lock and changed state before we did.
                    if (!cacheValid)
                    {
                        data = ...
                        cacheValid = true;
                    }
                }
                finally
                {
                    myRWLock.downgradeToReadLock();
                }
            }
            use(data);
        }
        finally
        {
            myRWLock.dropReadLock();
        }
    }
}
Henry HO
  • 121
  • 2
  • This is going to be prone to deadlocks. If one thread is inside upgradeToWriteLock() and another thread owns a read lock, the first thread will be blocking while waiting for the write lock but the other thread won't be able to call dropReadLock() since both methods are synchronized. – TrogDor Dec 03 '15 at 18:48
2

to OP: just unlock as many times as you have entered the lock, simple as that:

boolean needWrite = false;
readLock.lock()
try{
  needWrite = checkState();
}finally{
  readLock().unlock()
}

//the state is free to change right here, but not likely
//see who has handled it under the write lock, if need be
if (needWrite){
  writeLock().lock();
  try{
    if (checkState()){//check again under the exclusive write lock
   //modify state
    }
  }finally{
    writeLock.unlock()
  }
}

in the write lock as any self-respect concurrent program check the state needed.

HoldCount shouldn't be used beyond debug/monitor/fast-fail detect.

igr
  • 10,199
  • 13
  • 65
  • 111
xss
  • 31
  • 1
1

I suppose the ReentrantLock is motivated by a recursive traversal of the tree:

public void doSomething(Node node) {
  // Acquire reentrant lock
  ... // Do something, possibly acquire write lock
  for (Node child : node.childs) {
    doSomething(child);
  }
  // Release reentrant lock
}

Can't you refactor your code to move the lock handling outside of the recursion ?

public void doSomething(Node node) {
  // Acquire NON-reentrant read lock
  recurseDoSomething(node);
  // Release NON-reentrant read lock
}

private void recurseDoSomething(Node node) {
  ... // Do something, possibly acquire write lock
  for (Node child : node.childs) {
    recurseDoSomething(child);
  }
}
Olivier
  • 1,232
  • 6
  • 10
  • Not easily. There's a visitor pattern element to this meaning we lock the node for reading and then call out to other classes whose logic may or may not call back in pointing at the same node. Besides (in the more abstract sense) why make the lock reentrant if you can't use it safely that way? :-) – Andrzej Doyle Jan 21 '09 at 12:46
  • The idea is to acquire the read lock at client-invocation level. Your other classes could point back to the "internal" method (would probably become protected), knowing they are operating in the context of the read lock. – Olivier Jan 21 '09 at 13:24
  • True, I could do that, but then these methods are not inherently thread-safe. So far as possible I don't want to depend on the client needing to obtain a lock with subtly incorrect (yet often apparently correct) behaviour if they don't. – Andrzej Doyle Jan 22 '09 at 10:52
1

So, Are we expecting java to increment read semaphore count only if this thread has not yet contributed to the readHoldCount? Which means unlike just maintaining a ThreadLocal readholdCount of type int, It should maintain ThreadLocal Set of type Integer (maintaining the hasCode of current thread). If this is fine, I would suggest (at-least for now) not to call multiple read calls within the same class, but instead use a flag to check, whether read lock is already obtained by current object or not.

private volatile boolean alreadyLockedForReading = false;

public void lockForReading(Lock readLock){
   if(!alreadyLockedForReading){
      lock.getReadLock().lock();
   }
}
0

Found in the documentation for ReentrantReadWriteLock. It clearly says, that reader threads will never succeed when trying to acquire a write lock. What you try to achieve is simply not supported. You must release the read lock before acquisition of the write lock. A downgrade is still possible.

Reentrancy

This lock allows both readers and writers to reacquire read or write locks in the style of a {@link ReentrantLock}. Non-reentrant readers are not allowed until all write locks held by the writing thread have been released.

Additionally, a writer can acquire the read lock, but not vice-versa. Among other applications, reentrancy can be useful when write locks are held during calls or callbacks to methods that perform reads under read locks. If a reader tries to acquire the write lock it will never succeed.

Sample usage from the above source:

 class CachedData {
   Object data;
   volatile boolean cacheValid;
   ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();

   void processCachedData() {
     rwl.readLock().lock();
     if (!cacheValid) {
        // Must release read lock before acquiring write lock
        rwl.readLock().unlock();
        rwl.writeLock().lock();
        // Recheck state because another thread might have acquired
        //   write lock and changed state before we did.
        if (!cacheValid) {
          data = ...
          cacheValid = true;
        }
        // Downgrade by acquiring read lock before releasing write lock
        rwl.readLock().lock();
        rwl.writeLock().unlock(); // Unlock write, still hold read
     }

     use(data);
     rwl.readLock().unlock();
   }
 }
phi
  • 550
  • 8
  • 11
  • As I mentioned in the question, `I understand that with this particular class, one cannot elevate a read lock to a write lock, so as per the Javadocs one must release the read lock before obtaining the write lock.` My concern was entirely around **how** to reliably release the read lock, when it has been acquired in a reentrant fashion (and thus `rwl.readLock().unlock()` must be called a number of times greater than 1 - but how many?). – Andrzej Doyle Jul 02 '14 at 15:32
-1

Use the "fair" flag on the ReentrantReadWriteLock. "fair" means that lock requests are served on first come, first served. You could experience performance depredation since when you'll issue a "write" request, all of the subsequent "read" requests will be locked, even if they could have been served while the pre-existing read locks are still locked.

Ran Biron
  • 6,317
  • 5
  • 37
  • 67
  • 1
    I don't think you understand my problem, starvation is not an issue here. Try this in a single thread: ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true); lock.readLock().lock(); lock.readLock().lock(); lock.readLock().unlock(); lock.writeLock().lock(); It still blocks forever. – Andrzej Doyle Jan 21 '09 at 11:21