6712222: Race condition in java/lang/management/ThreadMXBean/AllThreadIds.java
authorjbachorik
Tue, 10 Mar 2015 09:37:56 +0100
changeset 29495 e0d12d371446
parent 29385 b3c34101dd61
child 29497 5e00b82f08cd
child 29498 fe656bc7c3f5
6712222: Race condition in java/lang/management/ThreadMXBean/AllThreadIds.java Reviewed-by: dholmes, dfuchs
jdk/test/java/lang/management/ThreadMXBean/AllThreadIds.java
--- a/jdk/test/java/lang/management/ThreadMXBean/AllThreadIds.java	Fri Mar 06 04:58:53 2015 -0800
+++ b/jdk/test/java/lang/management/ThreadMXBean/AllThreadIds.java	Tue Mar 10 09:37:56 2015 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,21 +27,19 @@
  * @summary Basic unit test of ThreadMXBean.getAllThreadIds()
  * @author  Alexei Guibadoulline and Mandy Chung
  *
- * @run build Barrier
  * @run main/othervm AllThreadIds
  */
 
 import java.lang.management.*;
-import java.util.*;
+import java.util.concurrent.Phaser;
 
 public class AllThreadIds {
     final static int DAEMON_THREADS = 20;
     final static int USER_THREADS = 5;
     final static int ALL_THREADS = DAEMON_THREADS + USER_THREADS;
-    private static volatile boolean live[] = new boolean[ALL_THREADS];
-    private static Thread allThreads[] = new Thread[ALL_THREADS];
-    private static ThreadMXBean mbean
-        = ManagementFactory.getThreadMXBean();
+    private static final boolean live[] = new boolean[ALL_THREADS];
+    private static final Thread allThreads[] = new Thread[ALL_THREADS];
+    private static final ThreadMXBean mbean = ManagementFactory.getThreadMXBean();
     private static boolean testFailed = false;
     private static boolean trace = false;
 
@@ -52,8 +50,7 @@
     private static int curLiveThreadCount = 0;
     private static int curPeakThreadCount = 0;
 
-    // barrier for threads communication
-    private static Barrier barrier = new Barrier(ALL_THREADS);
+    private static final Phaser startupCheck = new Phaser(ALL_THREADS + 1);
 
     private static void printThreadList() {
         if (!trace) return;
@@ -124,18 +121,15 @@
         curPeakThreadCount = mbean.getPeakThreadCount();
         checkThreadCount(0, 0);
 
-
         // Start all threads and wait to be sure they all are alive
-        barrier.set(ALL_THREADS);
         for (int i = 0; i < ALL_THREADS; i++) {
-            live[i] = true;
+            setLive(i, true);
             allThreads[i] = new MyThread(i);
-            allThreads[i].setDaemon( (i < DAEMON_THREADS) ? true : false);
+            allThreads[i].setDaemon(i < DAEMON_THREADS);
             allThreads[i].start();
         }
         // wait until all threads are started.
-        barrier.await();
-
+        startupCheck.arriveAndAwaitAdvance();
 
         checkThreadCount(ALL_THREADS, 0);
         printThreadList();
@@ -173,15 +167,14 @@
 
         // Stop daemon threads, wait to be sure they all are dead, and check
         // that they disappeared from getAllThreadIds() list
-        barrier.set(DAEMON_THREADS);
         for (int i = 0; i < DAEMON_THREADS; i++) {
-            live[i] = false;
+            setLive(i, false);
         }
-        // wait until daemon threads are terminated.
-        barrier.await();
 
-        // give chance to threads to terminate
-        pause();
+        // make sure the daemon threads are completely dead
+        joinDaemonThreads();
+
+        // and check the reported thread count
         checkThreadCount(0, DAEMON_THREADS);
 
         // Check mbean now
@@ -190,11 +183,11 @@
         for (int i = 0; i < ALL_THREADS; i++) {
             long expectedId = allThreads[i].getId();
             boolean found = false;
-            boolean live = (i >= DAEMON_THREADS);
+            boolean alive = (i >= DAEMON_THREADS);
 
             if (trace) {
                 System.out.print("Looking for thread with id " + expectedId +
-                    (live ? " expected alive." : " expected terminated."));
+                    (alive ? " expected alive." : " expected terminated."));
             }
             for (int j = 0; j < list.length; j++) {
                 if (expectedId == list[j]) {
@@ -203,11 +196,11 @@
                 }
             }
 
-            if (live != found) {
+            if (alive != found) {
                 testFailed = true;
             }
             if (trace) {
-                if (live != found) {
+                if (alive != found) {
                     System.out.println(" TEST FAILED.");
                 } else {
                     System.out.println();
@@ -216,15 +209,14 @@
         }
 
         // Stop all threads and wait to be sure they all are dead
-        barrier.set(ALL_THREADS - DAEMON_THREADS);
         for (int i = DAEMON_THREADS; i < ALL_THREADS; i++) {
-            live[i] = false;
+            setLive(i, false);
         }
-        // wait until daemon threads are terminated .
-        barrier.await();
 
-        // give chance to threads to terminate
-        pause();
+        // make sure the non-daemon threads are completely dead
+        joinNonDaemonThreads();
+
+        // and check the thread count
         checkThreadCount(0, ALL_THREADS - DAEMON_THREADS);
 
         if (testFailed)
@@ -233,6 +225,30 @@
         System.out.println("Test passed.");
     }
 
+    private static void joinDaemonThreads() throws InterruptedException {
+        for (int i = 0; i < DAEMON_THREADS; i++) {
+            allThreads[i].join();
+        }
+    }
+
+    private static void joinNonDaemonThreads() throws InterruptedException {
+        for (int i = DAEMON_THREADS; i < ALL_THREADS; i++) {
+            allThreads[i].join();
+        }
+    }
+
+    private static void setLive(int i, boolean val) {
+        synchronized(live) {
+            live[i] = val;
+        }
+    }
+
+    private static boolean isLive(int i) {
+        synchronized(live) {
+            return live[i];
+        }
+    }
+
     // The MyThread thread lives as long as correspondent live[i] value is true
     private static class MyThread extends Thread {
         int id;
@@ -243,8 +259,8 @@
 
         public void run() {
             // signal started
-            barrier.signal();
-            while (live[id]) {
+            startupCheck.arrive();
+            while (isLive(id)) {
                 try {
                     sleep(100);
                 } catch (InterruptedException e) {
@@ -253,23 +269,6 @@
                     testFailed = true;
                 }
             }
-            // signal about to exit
-            barrier.signal();
         }
     }
-
-    private static Object pauseObj = new Object();
-    private static void pause() {
-        // Enter lock a without blocking
-        synchronized (pauseObj) {
-            try {
-                // may need to tune this timeout for different platforms
-                pauseObj.wait(50);
-            } catch (Exception e) {
-                System.err.println("Unexpected exception.");
-                e.printStackTrace(System.err);
-            }
-        }
-    }
-
 }