javaconcurrencyreentrantreadwritelock

Is it safe to lock multiple ReentrantReadWriteLocks in the same try block?


Let's say I have two critial resources, foo and bar. I protect them with some ReentrantReadWriteLocks

ReentrantReadWriteLock foo = new RRWL() ...
ReentrantReadWriteLock bar = new RRWL() ...

Most operations only use foo OR bar, but some of them happen to use both. Now when using a single lock, you can't just do this:

void foo() {
   foo.writeLock().lock();
   privateWorkOnFoo();
   foo.writeLock().unlock();
}

If an exception is thrown, your foo will become forever locked. Instead you wrap it, like

void foo() {
    try {
        foo.writeLock().lock();
        privateWorkOnFoo();
    } finally { foo.writeLock().unlock(); }
}

But what if I need to work on both? Is it safe to put them in one block?

Option 1

try {
    foo.writeLock().lock();
    bar.writeLock().lock();
    magic();
} finally { 
    bar.writeLock().unlock();
    foo.writeLock().unlock();
}

Or is it necessary to give each lock its own block:

Option 2

try {
    foo.writeLock().lock();
    try {
        bar.writeLock().lock();
        magic();
    } finally { 
      bar.writeLock().unlock();
    }
    
} finally { 
    foo.writeLock().unlock();
}

I can't have been the first person to have hard to investigate this before... I know option 2 there is "bulletproof" but it's also a significant amount more maintenance. Is option 1 acceptable?


Solution

  • Option 1 is fine. It's known as the two lock variant. If you look at LinkedBlockingQueue operations such as remove, it locks the putLock as well as the takeLock. Here's a sample of what the JDK does:

      public boolean remove(Object o) {
           if (o == null) return false;
           fullyLock();
           try
           {
           // ...
           }   
           finally {
             fullyUnlock();
           }
        }
    
       /**
         * Lock to prevent both puts and takes.
         */
        void fullyLock() {
            putLock.lock();
            takeLock.lock();
        }
    
        /**
         * Unlock to allow both puts and takes.
         */
        void fullyUnlock() {
            takeLock.unlock();
            putLock.unlock();
        }