Multiple conditions vs Multiple locks

Question

For a particular thread-safe data structure, I am needed to protect access to a central data structure (namely a byte array). I am choosing to use ReentrantLocks in this case for it's fairness policy as well as advanced capabilities with creating multiple conditions.

The conditions for concurrency are complex and listed below:

1. The central byte array has to be protected exclusively (i.e. one thread at once).
2. Two accessing methods (foo and bar) must be able to run concurrently (blocking internally should they try to access the central byte array).
3. Calls to any method (foo and bar) need to be exclusive (i.e. multiple calls to foo from different threads will result in one thread blocking).

In my original implementation, I chose to implement two nested locks as below:

ReentrantLock lockFoo = new ReentrantLock(true);
ReentrantLock lockCentral = new ReentrantLock(true);

Condition centralCondition = lockCentral.newCondition();

public void foo(){
    // thread-safe processing code here

    lockFoo.lock();        
    lockCentral.lock();

    try{
        // accessing code here

        try{
            // waits upon some condition for access
            while(someCondition){
                centralCondition.await();
            }
        }catch(InterruptedException ex){
            // handling code here
        }

        // more processing
    }finally{
        lockCentral.unlock();
        lockFoo.unlock();
    }
}

the structure is equivalent in method bar, simply with another lock object lockBar. Additionally, the code has reduced my more complex multi-condition awaits and signals to a single condition for simplicity.

Using this, I can't help but feel the code seems unnecessarily complex and obscure in that not only that there are two locks nested, they share one try-finally, not to mention how lockCentral may be released and reacquired several times whilst lockFoo is held throughout.

Instead I tried to re-organize the outer lock (lockFoo and lockBar) as a condition of lockCentral instead, as below:

ReentrantLock lockCentral = new ReentrantLock(true);

Condition fooCondition = lockCentral.newCondition();
Condition centralCondition = lockCentral.newCondition();

boolean isInFoo = false;

public void foo(){
    // thread-safe processing code here

    lockCentral.lock();

    try{
        // implement method exclusiveness via fooCondition

        try{
            while(isInFoo){
                fooCondition.await();
            }

            isInFoo = true;
        }catch(InterruptedException ex){
            return;
        }

        // accessing code here

        try{
            // waits upon some condition for access
            while(someCondition){
                centralCondition.await();
            }
        }catch(InterruptedException ex){
            // handling code here
        }

        // more processing
    }finally{
        isInFoo = false;
        fooCondition.signal();

        lockCentral.unlock();
    }
}

after some inspection, I can't decide between whether the former is a better idea or the latter (especially with the inclusion of that random boolean). The idea of simplifying the code seems to have resulted in longer code, very counter-intuitive in this case.

Is there some convention or compelling reason to give argument to either:

1. Using one lock per locking context (the former code, where different reasons for locking share different locks).

2. Using one lock per locking resource (the latter code, where the central structure to be protected uses a single lock, everything else implemented as conditions to access said structure).

Answer 1

Latter code differs from the former one just by manual implementation of the lock lockFoo with fooCondition (the same is true for bar-related part).

Because such lock implementation takes into account, that foo critical section is almost same as central one, it is garantee to be faster in case when no contention on foo() (waiting on fooCondition is never performed in that case).

Aside from performance reasons, the former code is preferrable, as it is self-documented. Also, it is extendable to the case when data, protected by the lockFoo, are needed be accessed without lockCentral. In that case manual implementation of the lock loses its performance gain.

Answer 2

In your original example, the lockFoo and lockBar locks are completely redundant since neither foo() nor bar() can do any work without locking the lockCentral lock. Unless you change the design of the program, lockCentral is the only lock you need.

---

You said you thought your first example was "too complicated", but your second example is way more complicated. It looks as if you are merely trying to replace lockFoo and lockBar with locking code of your own design. But what's the point of that? It won't do anything different from what your first example does.

---

What is the purpose of the locking anyway? You said "Calls to any method (foo and bar) need to be exclusive". That's starting off on the wrong foot: Don't use locks to protect methods; Use locks to protec data.

What is this "central byte array?" What do the threads do to it? Why does it need to be protected?

What are the data on which foo() operates? Why does it need to be protected? What are the data on which bar() operates? Why does that need to be protected?

Most important of all, do the foo() data and the bar() data both need to be protected at the same time as the central byte array?

In a well designed program, threads should do most of their work without holding any locks:

SomeObject someObject;
SomeOtherObject someOtherObject;
boolean success = false;
while (! success) {

    someLock.lock();
    try {
        someObject = getLocalCopyOfSomeData();
        someOtherObject = getLocalCopyOfSomeOtherData();
    } finally {
        someLock.unlock();
    }

    doTheRealWork(someObject, someOtherObject);

    someLock.lock();
    try {
        success = updateCentralCopyOf(someObject) || updateCentralCopyOf(someOtherObject);
    } finally {
        someLock.unlock();
    }
}