Race condition in ThreadGroup stop test

[Gary Adams]

  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)
>>
>>
>>