6712222: Race condition in java/lang/management/ThreadMXBean/AllThreadIds.java
Reviewed-by: dholmes, dfuchs
--- 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);
- }
- }
- }
-
}