On 11/ 7/11 01:28 PM, chris hegarty wrote:
Hi Gary,
Thanks for taking this bug.
Alan was the one to suggest this as an easy starter 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! ).
Thanks I'll take a look at using Phaser as a replacement mechanism.
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!
One tweak to the notifyAll solution, it needs to be invoked
before the group stop is issued.
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)
