On 09/04/2013 04:24 AM, Mandy Chung wrote: > Hi Jaroslav, > > Like Daniel and David said, CyclicBarrier and other j.u.concurrent > utility seem a good replacement with the ThreadExecutionSynchronizer > class. ThreadMXBean/Locks.java was written prior to j.u.concurrent > added to the platform (both java.util.concurrent and > java.lang.management were added in JDK 5). Later > ThreadExecutionSynchronizer was added to fix some intermittent issue. > > I think it's worth the investigation to replace the existing logic with > j.u.concurrent and get rid of ThreadExecutionSynchronizer (which is used > by a few other tests).
Ok, let's go back to the basics :) The reason for the test to fail intermittently are stale reads from the "waiting" variable. In order to minimize the changes it seems sufficient to make the "waiting" variable volatile to prevent the stale reads. The code modifying the "waiting" variable is already guarded by the semaphore so we are good there. The changes in Locks.java are about more consistent approach to waiting for a thread to enter a desired state. I took Erik's recommendation to just wait indefinitely for the desired thread state and let the harness deal with timeouts. The very simplified patch is at http://cr.openjdk.java.net/~jbachorik/6815130/webrev.03 I will file a task for JDK9 to remove ThreadExecutionSynchronizer and simplify java.lang.management tests using it. -JB- > > Mandy > > On 9/3/2013 4:15 AM, 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- >
