Reputation: 8405
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:
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:
Using one lock per locking context (the former code, where different reasons for locking share different locks).
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).
Upvotes: 4
Views: 3135
Reputation: 27190
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();
}
}
Upvotes: 0
Reputation: 66089
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.
Upvotes: 5