Race condition in ThreadGroup stop test

[Rémi Forax]

On 11/07/2011 05:03 PM, 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.

Hi Gary,
groupStopped should be at least volatile or ...
it's perhaps better to use wait() instead of sleep(),
to use notifyAll() when groupStopped is equals to true and to use
a lock object to synchronize the wait and the notifyAll block.

something like

      private static Thread first=null;
      private static Thread second=null;
      private static ThreadGroup group = new ThreadGroup("");
+    private static boolean groupStopped =false ;
+    private static final Object lock = new Object();

      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)) {
+            synchronized(lock) {
+            groupStopped = true;
                     group.stop();
+                 lock.notifyAll();
+            }
+        }
              } catch(InterruptedException e){
              }
          }
@@ -57,7 +61,10 @@
      public static void main(String[] args) throws Exception {
          for (int i=0; i<2; i++)
              new Stop();
+    synchronized(lock) {
+      while(!groupStopped) {
            lock.wait();
+      }
+    }
+
          boolean failed = second.isAlive();
          first.stop(); second.stop();
          if (failed)


Rémi

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