Race condition in ThreadGroup stop test

[Brian Goetz]

I can recommend a good book....

Sent from my iPhone

On Nov 7, 2011, at 4:53 PM, Rémi Forax <forax at univ-mlv.fr> wrote:

> On 11/07/2011 09:50 PM, Brian Goetz wrote:
>> Wait/notify may be better than sleeping, but semaphore/latch/barrier are much better than wait/notify.  Much.
>> 
>> Sent from my iPhone
> 
> Yeah, APIs evolve, it seems I'm to old for that s... :)
> 
> Rémi
> 
>> 
>> On Nov 7, 2011, at 1:28 PM, chris hegarty<chris.hegarty at 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)
>>>> 
>>>> 
>>>> 
>