Skip to content

Commit 287bc67

Browse files
committed
Replace try/catch/propagate with try/finally idiom.
This actually fixes a serious bug: A few of those Throwable.propagate calls were potentially wrapping InterruptedExceptions. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=25813299
1 parent c72ba76 commit 287bc67

File tree

1 file changed

+54
-76
lines changed

1 file changed

+54
-76
lines changed

guava/src/com/google/common/util/concurrent/Monitor.java

+54-76
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,15 @@ public void enterWhen(Guard guard) throws InterruptedException {
360360
}
361361
final ReentrantLock lock = this.lock;
362362
boolean reentrant = lock.isHeldByCurrentThread();
363+
boolean success = false;
363364
lock.lockInterruptibly();
364365
try {
365366
waitInterruptibly(guard, reentrant);
366-
} catch (Throwable throwable) {
367-
lock.unlock();
368-
throw Throwables.propagate(throwable);
367+
success = true;
368+
} finally {
369+
if (!success) {
370+
lock.unlock();
371+
}
369372
}
370373
}
371374

@@ -378,12 +381,15 @@ public void enterWhenUninterruptibly(Guard guard) {
378381
}
379382
final ReentrantLock lock = this.lock;
380383
boolean reentrant = lock.isHeldByCurrentThread();
384+
boolean success = false;
381385
lock.lock();
382386
try {
383387
waitUninterruptibly(guard, reentrant);
384-
} catch (Throwable throwable) {
385-
lock.unlock();
386-
throw Throwables.propagate(throwable);
388+
success = true;
389+
} finally {
390+
if (!success) {
391+
lock.unlock();
392+
}
387393
}
388394
}
389395

@@ -404,20 +410,16 @@ public boolean enterWhen(Guard guard, long time, TimeUnit unit) throws Interrupt
404410
if (!lock.tryLock(time, unit)) {
405411
return false;
406412
}
407-
boolean satisfied;
413+
boolean satisfied = false;
408414
try {
409415
long remainingNanos = unit.toNanos(time) - (System.nanoTime() - startNanos);
410416
satisfied = waitInterruptibly(guard, remainingNanos, reentrant);
411-
} catch (Throwable throwable) {
412-
lock.unlock();
413-
throw Throwables.propagate(throwable);
414-
}
415-
if (satisfied) {
416-
return true;
417-
} else {
418-
lock.unlock();
419-
return false;
417+
} finally {
418+
if (!satisfied) {
419+
lock.unlock();
420+
}
420421
}
422+
return satisfied;
421423
}
422424

423425
/**
@@ -450,19 +452,15 @@ public boolean enterWhenUninterruptibly(Guard guard, long time, TimeUnit unit) {
450452
remainingNanos = (timeoutNanos - (System.nanoTime() - startNanos));
451453
}
452454
}
453-
boolean satisfied;
455+
boolean satisfied = false;
454456
try {
455457
satisfied = waitUninterruptibly(guard, remainingNanos, reentrant);
456-
} catch (Throwable throwable) {
457-
lock.unlock();
458-
throw Throwables.propagate(throwable);
459-
}
460-
if (satisfied) {
461-
return true;
462-
} else {
463-
lock.unlock();
464-
return false;
458+
} finally {
459+
if (!satisfied) {
460+
lock.unlock();
461+
}
465462
}
463+
return satisfied;
466464
} finally {
467465
if (interruptIgnored) {
468466
Thread.currentThread().interrupt();
@@ -482,19 +480,15 @@ public boolean enterIf(Guard guard) {
482480
}
483481
final ReentrantLock lock = this.lock;
484482
lock.lock();
485-
boolean satisfied;
483+
boolean satisfied = false;
486484
try {
487485
satisfied = guard.isSatisfied();
488-
} catch (Throwable throwable) {
489-
lock.unlock();
490-
throw Throwables.propagate(throwable);
491-
}
492-
if (satisfied) {
493-
return true;
494-
} else {
495-
lock.unlock();
496-
return false;
486+
} finally {
487+
if (!satisfied) {
488+
lock.unlock();
489+
}
497490
}
491+
return satisfied;
498492
}
499493

500494
/**
@@ -509,19 +503,15 @@ public boolean enterIfInterruptibly(Guard guard) throws InterruptedException {
509503
}
510504
final ReentrantLock lock = this.lock;
511505
lock.lockInterruptibly();
512-
boolean satisfied;
506+
boolean satisfied = false;
513507
try {
514508
satisfied = guard.isSatisfied();
515-
} catch (Throwable throwable) {
516-
lock.unlock();
517-
throw Throwables.propagate(throwable);
518-
}
519-
if (satisfied) {
520-
return true;
521-
} else {
522-
lock.unlock();
523-
return false;
509+
} finally {
510+
if (!satisfied) {
511+
lock.unlock();
512+
}
524513
}
514+
return satisfied;
525515
}
526516

527517
/**
@@ -538,19 +528,15 @@ public boolean enterIf(Guard guard, long time, TimeUnit unit) {
538528
if (!enter(time, unit)) {
539529
return false;
540530
}
541-
boolean satisfied;
531+
boolean satisfied = false;
542532
try {
543533
satisfied = guard.isSatisfied();
544-
} catch (Throwable throwable) {
545-
lock.unlock();
546-
throw Throwables.propagate(throwable);
547-
}
548-
if (satisfied) {
549-
return true;
550-
} else {
551-
lock.unlock();
552-
return false;
534+
} finally {
535+
if (!satisfied) {
536+
lock.unlock();
537+
}
553538
}
539+
return satisfied;
554540
}
555541

556542
/**
@@ -568,19 +554,15 @@ public boolean enterIfInterruptibly(Guard guard, long time, TimeUnit unit)
568554
if (!lock.tryLock(time, unit)) {
569555
return false;
570556
}
571-
boolean satisfied;
557+
boolean satisfied = false;
572558
try {
573559
satisfied = guard.isSatisfied();
574-
} catch (Throwable throwable) {
575-
lock.unlock();
576-
throw Throwables.propagate(throwable);
577-
}
578-
if (satisfied) {
579-
return true;
580-
} else {
581-
lock.unlock();
582-
return false;
560+
} finally {
561+
if (!satisfied) {
562+
lock.unlock();
563+
}
583564
}
565+
return satisfied;
584566
}
585567

586568
/**
@@ -599,19 +581,15 @@ public boolean tryEnterIf(Guard guard) {
599581
if (!lock.tryLock()) {
600582
return false;
601583
}
602-
boolean satisfied;
584+
boolean satisfied = false;
603585
try {
604586
satisfied = guard.isSatisfied();
605-
} catch (Throwable throwable) {
606-
lock.unlock();
607-
throw Throwables.propagate(throwable);
608-
}
609-
if (satisfied) {
610-
return true;
611-
} else {
612-
lock.unlock();
613-
return false;
587+
} finally {
588+
if (!satisfied) {
589+
lock.unlock();
590+
}
614591
}
592+
return satisfied;
615593
}
616594

617595
/**

0 commit comments

Comments
 (0)