Hi Jaroslav,
On 3/09/2013 11:02 PM, Jaroslav Bachorik wrote:
On 09/03/2013 02:10 PM, Daniel Fuchs wrote:
Hi Jaroslav,
Have you considered replacing the ThreadExecutionSynchronizer with
a plain (and more reliable) CyclicBarrier or Phaser object?
<http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CyclicBarrier.html>
<http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Phaser.html>
Yes, I've played with the idea of using more advanced concurrency
constructs. But I couldn't fit the CyclicBarrier to the usecase and when
I used Phaser the solution complexity was considerably higher. It might
be due to my lack of experience with the Phaser, specifically, but the
plain old 'synchronized' yielded the best results for me :(
Unfortunately your use of wait() does not account for spurious wakeups -
which the existing Semaphore masks from you.
A CyclicBarrier would seem to be a better fit, but the logic in this
test is convoluted enough that it is hard to tell exactly what the
synchronization protocol is.
Also your gratuitous style changes made it very hard to see actual
functional changes in the test. There's really no need to change for
loops into for-each loops. ;-)
David
----
-JB-
best regards
-- daniel
On 9/3/13 1:15 PM, Jaroslav Bachorik wrote:
Please, review the following patch of the intermittently failing test:
http://cr.openjdk.java.net/~jbachorik/6815130/webrev.01
Issue: https://bugs.openjdk.java.net/browse/JDK-6815130
Sometimes the ThreadExecutionSynchronizer class failes to achieve the
desired synchronization (due to possible data races when evaluating and
setting the "waiting" variable) leading to test failures.
The patch fixes the possibility of data race. Also the Locks class is
tidied up a bit.
Thanks,
-JB-
