SUGGESTED FIX
--- /u/martin/ws/mustang/src/share/classes/java/util/concurrent/CyclicBarrier.java 2005-12-04 14:55:05.651137000 -0800
+++ /u/martin/ws/holmes/src/share/classes/java/util/concurrent/CyclicBarrier.java 2006-01-03 12:49:53.981494000 -0800
@@ -115,7 +115,6 @@
*/
private static class Generation {
boolean broken = false;
- boolean tripped = false;
}
/** The lock for guarding barrier entry */
@@ -142,7 +141,6 @@
*/
private void nextGeneration() {
// signal completion of last generation
- generation.tripped = true;
trip.signalAll();
// set up next generation
count = parties;
@@ -202,14 +200,21 @@
else if (nanos > 0L)
nanos = trip.awaitNanos(nanos);
} catch (InterruptedException ie) {
+ if (g == generation && ! g.broken) {
breakBarrier();
throw ie;
+ } else {
+ // We're about to finish waiting even if we had not
+ // been interrupted, so this interrupt is deemed to
+ // "belong" to subsequent execution.
+ Thread.currentThread().interrupt();
+ }
}
if (g.broken)
throw new BrokenBarrierException();
- if (g.tripped)
+ if (g != generation)
return index;
if (timed && nanos <= 0L) {
@@ -270,7 +275,7 @@
*
* <p>If the current thread is not the last to arrive then it is
* disabled for thread scheduling purposes and lies dormant until
- * one of following things happens:
+ * one of the following things happens:
* <ul>
* <li>The last thread arrives; or
* <li>Some other thread {@link Thread#interrupt interrupts} the current
|
EVALUATION
Two refinements:
- the "tripped" field can be eliminated, since g.tripped is equivalent to "generation == g"
- If an InterruptedException is caught when the await would have ended for some other reason
anyways, the interrupt bit should be set on again, and the wait ended for the other reason,
possibly throwing BrokenBarrierException. This leads to slightly more predictable behavior.
(It's still not a good idea to try to break a CyclicBarrier by interrupting more than one thread)
|