Re: [infinispan-dev] findbugs found issue
On 09/03/2009 09:38 AM, David M. Lloyd wrote:
On 09/03/2009 03:55 AM, Mircea Markus wrote:
> Hi,
>
> findbugs detects a problem in the following code:
>
> public void clear() {
> // This is expensive...
> // lock all segments
> for (Segment s : segments) s.lock();
> try {
> for (Segment s : segments) s.locklessClear();
> initLinks();
> } finally {
> for (Segment s : segments) s.unlock();
> }
> }
> In my eyes the code is okay, am I missing something?
Yes - if one of the unlocks fail, the subsequent locks will not be
released. If one of the locks fail, none of the acquired locks will be
released.
Also you're unlocking in the same order which you lock, which is begging
for deadlocks.
A safer approach would be to use recursion:
private void clear(Iterator<Segment> i) {
if (i.hasNext()) {
Segment s = i.next();
s.lock();
try {
clear(i);
} finally {
s.unlock();
}
} else {
for (Segment s : segments) s.locklessClear();
initLinks();
}
}
This ensures that (a) the locks are released in the reverse order they were
acquired in, and (b) if any lock or unlock operation fails, things don't go
haywire. Of course if you have hundreds or thousands of segments, you risk
blowing out the stack.
- DML