Wait/notify may be better than sleeping, but semaphore/latch/barrier are much better than wait/notify. Much.
Sent from my iPhone On Nov 7, 2011, at 1:28 PM, chris hegarty <chris.hega...@oracle.com> wrote: > Hi Gary, > > Thanks for taking this bug. > > In similar tests I've preferred to use j.u.c.Paser/CountDownLatch to > coordinate between threads. So, you could completely remove the sleep(1000) > from the run method and use a Phaser.arriveAndAwaitAdvance(). This will > guarantee both threads will reach a particular point before invoking stop on > the group. That way, either thread can invoke stop and set the flag to > prevent the other thread from invoking stop too ( if it were to get there! ). > > I agree with Rémi's comments about wait/notify. My comments (above) should > work with well with his suggestion. Then no more nasty sleeps! > > Thanks, > -Chris. > > On 07/11/2011 16:03, Gary Adams wrote: >> >> >> Here's another test with race conditions built into the test >> that can be easily avoided >> >> CR#7084033 - TEST_BUG: test/java/lang/ThreadGroup/Stop.java fails >> intermittently >> >> There are at least two race conditions in the test as currently written. >> The test uses sleeps as a way to yield time to other threads to complete >> their >> tasks, but this may not be sufficient on slower machines. >> >> The first race condition is in the starting of the test threads. >> To ensure both threads are started, the run check for the first thread >> should >> also check that the second thread is not null. >> >> The second race condition in the main thread presumes that >> the group stop has been issued within 3000 milliseconds. >> A simple loop on the delay can be gated by a flag set after the group >> stop has been issued. >> >> diff --git a/test/java/lang/ThreadGroup/Stop.java >> b/test/java/lang/ThreadGroup/Stop.java >> --- a/test/java/lang/ThreadGroup/Stop.java >> +++ b/test/java/lang/ThreadGroup/Stop.java >> @@ -32,6 +32,7 @@ >> private static Thread first=null; >> private static Thread second=null; >> private static ThreadGroup group = new ThreadGroup(""); >> + private static boolean groupStopped =false ; >> >> Stop() { >> Thread thread = new Thread(group, this); >> @@ -47,8 +48,11 @@ >> while (true) { >> try { >> Thread.sleep(1000); // Give other thread a chance to start >> - if (Thread.currentThread() == first) >> + if ((Thread.currentThread() == first) >> + && (second != null)) { >> + groupStopped = true; >> group.stop(); >> + } >> } catch(InterruptedException e){ >> } >> } >> @@ -57,7 +61,10 @@ >> public static void main(String[] args) throws Exception { >> for (int i=0; i<2; i++) >> new Stop(); >> + do { >> Thread.sleep(3000); >> + } while (!groupStopped) ; >> + >> boolean failed = second.isAlive(); >> first.stop(); second.stop(); >> if (failed) >> >> >>