8309406: Change jdk.trackAllThreads to default to true

Reviewed-by: rpressler, mchung, cjplummer

src/java.base/share/classes/jdk/internal/vm/SharedThreadContainer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2023, 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
@@ -29,7 +29,6 @@ import java.lang.invoke.VarHandle;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.LongAdder;
 import java.util.stream.Stream;
 import jdk.internal.access.JavaLangAccess;
 import jdk.internal.access.SharedSecrets;
@@ -50,16 +49,13 @@ public class SharedThreadContainer extends ThreadContainer implements AutoClosea
             VIRTUAL_THREADS = l.findVarHandle(SharedThreadContainer.class,
                     "virtualThreads", Set.class);
         } catch (Exception e) {
-            throw new InternalError(e);
+            throw new ExceptionInInitializerError(e);
         }
     }
 
     // name of container, used by toString
     private final String name;
 
-    // the number of threads in the container
-    private final LongAdder threadCount;
-
     // the virtual threads in the container, created lazily
     private volatile Set<Thread> virtualThreads;
 
@@ -76,7 +72,6 @@ public class SharedThreadContainer extends ThreadContainer implements AutoClosea
     private SharedThreadContainer(String name) {
         super(/*shared*/ true);
         this.name = name;
-        this.threadCount = new LongAdder();
     }
 
     /**
@@ -119,21 +114,14 @@ public class SharedThreadContainer extends ThreadContainer implements AutoClosea
             }
             vthreads.add(thread);
         }
-        threadCount.add(1L);
     }
 
     @Override
     public void onExit(Thread thread) {
-        threadCount.add(-1L);
         if (thread.isVirtual())
             virtualThreads.remove(thread);
     }
 
-    @Override
-    public long threadCount() {
-        return threadCount.sum();
-    }
-
     @Override
     public Stream<Thread> threads() {
         // live platform threads in this container

src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java

@@ -33,20 +33,35 @@ import java.util.concurrent.atomic.LongAdder;
 import java.util.stream.Stream;
 import jdk.internal.access.JavaLangAccess;
 import jdk.internal.access.SharedSecrets;
-import sun.nio.ch.Poller;
 import sun.security.action.GetPropertyAction;
 
 /**
- * This class consists exclusively of static methods to support debugging and
+ * This class consists exclusively of static methods to support groupings of threads.
- * monitoring of threads.
  */
 public class ThreadContainers {
     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
 
+    // true if all threads are tracked
+    private static final boolean TRACK_ALL_THREADS;
+
+    // the root container
+    private static final RootContainer ROOT_CONTAINER;
+
     // the set of thread containers registered with this class
     private static final Set<WeakReference<ThreadContainer>> CONTAINER_REGISTRY = ConcurrentHashMap.newKeySet();
     private static final ReferenceQueue<Object> QUEUE = new ReferenceQueue<>();
 
+    static {
+        String s = GetPropertyAction.privilegedGetProperty("jdk.trackAllThreads");
+        if (s == null || s.isEmpty() || Boolean.parseBoolean(s)) {
+            TRACK_ALL_THREADS = true;
+            ROOT_CONTAINER = new RootContainer.TrackingRootContainer();
+        } else {
+            TRACK_ALL_THREADS = false;
+            ROOT_CONTAINER = new RootContainer.CountingRootContainer();
+        }
+    }
+
     private ThreadContainers() { }
 
     /**
@@ -59,6 +74,13 @@ public class ThreadContainers {
         }
     }
 
+    /**
+     * Returns true if all threads are tracked.
+     */
+    public static boolean trackAllThreads() {
+        return TRACK_ALL_THREADS;
+    }
+
     /**
      * Registers a thread container to be tracked this class, returning a key
      * that is used to remove it from the registry.
@@ -83,7 +105,7 @@ public class ThreadContainers {
      * Returns the root thread container.
      */
     public static ThreadContainer root() {
-        return RootContainer.INSTANCE;
+        return ROOT_CONTAINER;
     }
 
     /**
@@ -183,20 +205,10 @@ public class ThreadContainers {
     }
 
     /**
-     * Root container that "contains" all platform threads not started in a
+     * Root container that "contains" all platform threads not started in a container.
-     * container plus some (or all) virtual threads that are started directly
+     * It may include all virtual threads started directly with the Thread API.
-     * with the Thread API.
      */
     private static abstract class RootContainer extends ThreadContainer {
-        static final RootContainer INSTANCE;
-        static {
-            String s = GetPropertyAction.privilegedGetProperty("jdk.trackAllThreads");
-            if (s != null && (s.isEmpty() || Boolean.parseBoolean(s))) {
-                INSTANCE = new TrackingRootContainer();
-            } else {
-                INSTANCE = new CountingRootContainer();
-            }
-        }
         protected RootContainer() {
             super(true);
         }
@@ -270,11 +282,7 @@ public class ThreadContainers {
             }
             @Override
             public Stream<Thread> threads() {
-                // virtual threads in this container that are those blocked on I/O.
+                return platformThreads();
-                Stream<Thread> blockedVirtualThreads = Poller.blockedThreads()
-                        .filter(t -> t.isVirtual()
-                                && JLA.threadContainer(t) == this);
-                return Stream.concat(platformThreads(), blockedVirtualThreads);
             }
         }
     }

src/java.base/share/classes/jdk/internal/vm/ThreadDumper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2023, 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
@@ -240,7 +240,9 @@ public class ThreadDumper {
         out.println("        ],");   // end of threads
 
         // thread count
-        threadCount = Long.max(threadCount, container.threadCount());
+        if (!ThreadContainers.trackAllThreads()) {
+            threadCount = Long.max(threadCount, container.threadCount());
+        }
         out.format("        \"threadCount\": \"%d\"%n", threadCount);
 
         if (more) {

test/hotspot/jtreg/serviceability/dcmd/thread/ThreadDumpToFileTest.java

@@ -66,7 +66,7 @@ class ThreadDumpToFileTest {
     @Test
     void testJsonThreadDump() throws IOException {
         Path file = genThreadDumpPath(".json");
-        threadDump(file, "-format=json").shouldMatch("Created");
+        jcmdThreadDumpToFile(file, "-format=json").shouldMatch("Created");
 
         // parse the JSON text
         String jsonText = Files.readString(file);
@@ -89,7 +89,7 @@ class ThreadDumpToFileTest {
         Path file = genThreadDumpPath(".txt");
         Files.writeString(file, "xxx");
 
-        threadDump(file, "").shouldMatch("exists");
+        jcmdThreadDumpToFile(file, "").shouldMatch("exists");
 
         // file should not be overridden
         assertEquals("xxx", Files.readString(file));
@@ -102,14 +102,14 @@ class ThreadDumpToFileTest {
     void testOverwriteFile() throws IOException {
         Path file = genThreadDumpPath(".txt");
         Files.writeString(file, "xxx");
-        testPlainThreadDump(file, "-overwrite");
+        jcmdThreadDumpToFile(file, "-overwrite");
     }
 
     /**
      * Test thread dump in plain text format.
      */
     private void testPlainThreadDump(Path file, String... options) throws IOException {
-        threadDump(file, options).shouldMatch("Created");
+        jcmdThreadDumpToFile(file, options).shouldMatch("Created");
 
         // test that thread dump contains the name and id of the current thread
         String name = Thread.currentThread().getName();
@@ -118,6 +118,9 @@ class ThreadDumpToFileTest {
         assertTrue(find(file, expected), expected + " not found in " + file);
     }
 
+    /**
+     * Generate a file path with the given suffix to use for the thread dump.
+     */
     private Path genThreadDumpPath(String suffix) throws IOException {
         Path dir = Path.of(".").toAbsolutePath();
         Path file = Files.createTempFile(dir, "threads-", suffix);
@@ -125,7 +128,10 @@ class ThreadDumpToFileTest {
         return file;
     }
 
-    private OutputAnalyzer threadDump(Path file, String... options) {
+    /**
+     * Launches jcmd Thread.dump_to_file to obtain a thread dump of this VM.
+     */
+    private OutputAnalyzer jcmdThreadDumpToFile(Path file, String... options) {
         String cmd = "Thread.dump_to_file";
         for (String option : options) {
             cmd += " " + option;
@@ -133,6 +139,9 @@ class ThreadDumpToFileTest {
         return new PidJcmdExecutor().execute(cmd + " " + file);
     }
 
+    /**
+     * Returns true if the given file contains a line with the string.
+     */
     private boolean find(Path file, String text) throws IOException {
         try (Stream<String> stream = Files.lines(file)) {
             return  stream.anyMatch(line -> line.indexOf(text) >= 0);

test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java

@@ -23,23 +23,28 @@
 
 /**
  * @test
- * @bug 8284161 8287008
+ * @bug 8284161 8287008 8309406
  * @summary Basic test for com.sun.management.HotSpotDiagnosticMXBean.dumpThreads
+ * @requires vm.continuations
+ * @modules jdk.management
  * @library /test/lib
  * @run junit/othervm DumpThreads
  * @run junit/othervm -Djdk.trackAllThreads DumpThreads
  * @run junit/othervm -Djdk.trackAllThreads=true DumpThreads
- * @run junit/othervm -Djdk.trackAllThreadds=false DumpThreads
+ * @run junit/othervm -Djdk.trackAllThreads=false DumpThreads
  */
 
 import java.lang.management.ManagementFactory;
 import java.nio.file.Files;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Path;
+import java.util.Objects;
 import java.time.ZonedDateTime;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.locks.LockSupport;
 import java.util.stream.Stream;
 import com.sun.management.HotSpotDiagnosticMXBean;
@@ -47,104 +52,201 @@ import com.sun.management.HotSpotDiagnosticMXBean.ThreadDumpFormat;
 import jdk.test.lib.threaddump.ThreadDump;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import static org.junit.jupiter.api.Assertions.*;
 
 class DumpThreads {
-    private static final boolean TRACK_ALL_THREADS;
+    private static boolean trackAllThreads;
-    static {
+
+    @BeforeAll
+    static void setup() throws Exception {
         String s = System.getProperty("jdk.trackAllThreads");
-        TRACK_ALL_THREADS = (s != null) && (s.isEmpty() || Boolean.parseBoolean(s));
+        trackAllThreads = (s == null) || s.isEmpty() || Boolean.parseBoolean(s);
     }
 
     /**
-     * Thread dump in plain text format.
+     * ExecutorService implementations that have their object identity in the container
+     * name so they can be found in the JSON format.
+     */
+    static Stream<ExecutorService> executors() {
+        return Stream.of(
+                Executors.newFixedThreadPool(1),
+                Executors.newVirtualThreadPerTaskExecutor()
+        );
+    }
+
+    /**
+     * Test thread dump in plain text format contains information about the current
+     * thread and a virtual thread created directly with the Thread API.
      */
     @Test
-    void testPlainText() throws Exception {
+    void testRootContainerPlainTextFormat() throws Exception {
-        var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class);
+        Thread vthread = Thread.ofVirtual().start(LockSupport::park);
-        Path file = genOutputPath("txt");
+        try {
-        try (var executor = Executors.newVirtualThreadPerTaskExecutor()) {
+            testDumpThreadsPlainText(vthread, trackAllThreads);
-            Thread vthread = forkParker(executor);
-            try {
-                mbean.dumpThreads(file.toString(), ThreadDumpFormat.TEXT_PLAIN);
-                cat(file);
-
-                // pid should be on the first line
-                String pid = "" + ProcessHandle.current().pid();
-                assertTrue(line(file, 0).contains(pid));
-
-                // runtime version should be on third line
-                String vs = Runtime.version().toString();
-                assertTrue(line(file, 2).contains(vs));
-
-                // virtual thread should be found
-                assertTrue(isPresent(file, vthread));
-
-                // if the current thread is a platform thread then it should be included
-                Thread currentThread = Thread.currentThread();
-                if (!currentThread.isVirtual() || TRACK_ALL_THREADS) {
-                    assertTrue(isPresent(file, currentThread));
-                }
-            } finally {
-                LockSupport.unpark(vthread);
-            }
         } finally {
-            Files.deleteIfExists(file);
+            LockSupport.unpark(vthread);
         }
     }
 
     /**
-     * Thread dump in JSON format.
+     * Test thread dump in JSON format contains information about the current
+     * thread and a virtual thread created directly with the Thread API.
      */
     @Test
-    void testJson() throws Exception {
+    void testRootContainerJsonFormat() throws Exception {
-        var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class);
+        Thread vthread = Thread.ofVirtual().start(LockSupport::park);
-        Path file = genOutputPath("json");
+        try {
-        try (var executor = Executors.newVirtualThreadPerTaskExecutor()) {
+            testDumpThreadsJson(null, vthread, trackAllThreads);
-            Thread vthread = forkParker(executor);
-            try {
-                mbean.dumpThreads(file.toString(), ThreadDumpFormat.JSON);
-                cat(file);
-
-                // parse the JSON text
-                String jsonText = Files.readString(file);
-                ThreadDump threadDump = ThreadDump.parse(jsonText);
-
-                // test threadDump/processId
-                assertTrue(threadDump.processId() == ProcessHandle.current().pid());
-
-                // test threadDump/time can be parsed
-                ZonedDateTime.parse(threadDump.time());
-
-                // test threadDump/runtimeVersion
-                assertEquals(Runtime.version().toString(), threadDump.runtimeVersion());
-
-                // test root container
-                var rootContainer = threadDump.rootThreadContainer();
-                assertFalse(rootContainer.owner().isPresent());
-                assertFalse(rootContainer.parent().isPresent());
-
-                // if the current thread is a platform thread then it will be in root container
-                Thread currentThread = Thread.currentThread();
-                if (!currentThread.isVirtual() || TRACK_ALL_THREADS) {
-                    rootContainer.findThread(currentThread.threadId()).orElseThrow();
-                }
-
-                // find the thread container for the executor. The name of this executor
-                // is its String representaiton in this case.
-                String name = executor.toString();
-                var container = threadDump.findThreadContainer(name).orElseThrow();
-                assertFalse(container.owner().isPresent());
-                assertTrue(container.parent().get() == rootContainer);
-                container.findThread(vthread.threadId()).orElseThrow();
-            } finally {
-                LockSupport.unpark(vthread);
-            }
         } finally {
-            Files.deleteIfExists(file);
+            LockSupport.unpark(vthread);
         }
     }
 
+    /**
+     * Test thread dump in plain text format includes a thread executing a task in the
+     * given ExecutorService.
+     */
+    @ParameterizedTest
+    @MethodSource("executors")
+    void testExecutorServicePlainTextFormat(ExecutorService executor) throws Exception {
+        try (executor) {
+            Thread thread = forkParker(executor);
+            try {
+                testDumpThreadsPlainText(thread, true);
+            } finally {
+                LockSupport.unpark(thread);
+            }
+        }
+    }
+
+    /**
+     * Test thread dump in JSON format includes a thread executing a task in the
+     * given ExecutorService.
+     */
+    @ParameterizedTest
+    @MethodSource("executors")
+    void testExecutorServiceJsonFormat(ExecutorService executor) throws Exception {
+        try (executor) {
+            Thread thread = forkParker(executor);
+            try {
+                testDumpThreadsJson(Objects.toIdentityString(executor), thread, true);
+            } finally {
+                LockSupport.unpark(thread);
+            }
+        }
+    }
+
+    /**
+     * Test thread dump in JSON format includes a thread executing a task in the
+     * fork-join common pool.
+     */
+    @Test
+    void testForkJoinPool() throws Exception {
+        ForkJoinPool pool = ForkJoinPool.commonPool();
+        Thread thread = forkParker(pool);
+        try {
+            testDumpThreadsJson("ForkJoinPool.commonPool", thread, true);
+        } finally {
+            LockSupport.unpark(thread);
+        }
+    }
+
+    /**
+     * Invoke HotSpotDiagnosticMXBean.dumpThreads to create a thread dump in plain text
+     * format, then sanity check that the thread dump includes expected strings, the
+     * current thread, and maybe the given thread.
+     * @param thread the thread to test if included
+     * @param expectInDump true if the thread is expected to be included
+     */
+    private void testDumpThreadsPlainText(Thread thread, boolean expectInDump) throws Exception {
+        Path file = genOutputPath(".txt");
+        var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class);
+        mbean.dumpThreads(file.toString(), ThreadDumpFormat.TEXT_PLAIN);
+        System.err.format("Dumped to %s%n", file);
+
+        // pid should be on the first line
+        String line1 = line(file, 0);
+        String pid = Long.toString(ProcessHandle.current().pid());
+        assertTrue(line1.contains(pid));
+
+        // timestamp should be on the second line
+        String line2 = line(file, 1);
+        ZonedDateTime.parse(line2);
+
+        // runtime version should be on third line
+        String line3 = line(file, 2);
+        String vs = Runtime.version().toString();
+        assertTrue(line3.contains(vs));
+
+        // test if thread is included in thread dump
+        assertEquals(expectInDump, isPresent(file, thread));
+
+        // current thread should be included if platform thread or tracking all threads
+        Thread currentThread = Thread.currentThread();
+        boolean currentThreadExpected = trackAllThreads || !currentThread.isVirtual();
+        assertEquals(currentThreadExpected, isPresent(file, currentThread));
+    }
+
+    /**
+     * Invoke HotSpotDiagnosticMXBean.dumpThreads to create a thread dump in JSON format.
+     * The thread dump is parsed as a JSON object and checked to ensure that it contains
+     * expected data, the current thread, and maybe the given thread.
+     * @param containerName the name of the container or null for the root container
+     * @param thread the thread to test if included
+     * @param expect true if the thread is expected to be included
+     */
+    private void testDumpThreadsJson(String containerName,
+                                     Thread thread,
+                                     boolean expectInDump) throws Exception {
+        Path file = genOutputPath(".json");
+        var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class);
+        mbean.dumpThreads(file.toString(), ThreadDumpFormat.JSON);
+        System.err.format("Dumped to %s%n", file);
+
+        // parse the JSON text
+        String jsonText = Files.readString(file);
+        ThreadDump threadDump = ThreadDump.parse(jsonText);
+
+        // test threadDump/processId
+        assertTrue(threadDump.processId() == ProcessHandle.current().pid());
+
+        // test threadDump/time can be parsed
+        ZonedDateTime.parse(threadDump.time());
+
+        // test threadDump/runtimeVersion
+        assertEquals(Runtime.version().toString(), threadDump.runtimeVersion());
+
+        // test root container, has no parent and no owner
+        var rootContainer = threadDump.rootThreadContainer();
+        assertFalse(rootContainer.owner().isPresent());
+        assertFalse(rootContainer.parent().isPresent());
+
+        // test that the container contains the given thread
+        ThreadDump.ThreadContainer container;
+        if (containerName == null) {
+            // root container, the thread should be found if trackAllThreads is true
+            container = rootContainer;
+        } else {
+            // find the container
+            container = threadDump.findThreadContainer(containerName).orElse(null);
+            assertNotNull(container, containerName + " not found");
+            assertFalse(container.owner().isPresent());
+            assertTrue(container.parent().get() == rootContainer);
+
+        }
+        boolean found = container.findThread(thread.threadId()).isPresent();
+        assertEquals(expectInDump, found);
+
+        // current thread should be in root container if platform thread or tracking all threads
+        Thread currentThread = Thread.currentThread();
+        boolean currentThreadExpected = trackAllThreads || !currentThread.isVirtual();
+        found = rootContainer.findThread(currentThread.threadId()).isPresent();
+        assertEquals(currentThreadExpected, found);
+    }
+
     /**
      * Test that dumpThreads throws if the output file already exists.
      */
@@ -186,21 +288,24 @@ class DumpThreads {
      * Submits a parking task to the given executor, returns the Thread object of
      * the parked thread.
      */
-    private static Thread forkParker(ExecutorService executor) throws Exception {
+    private static Thread forkParker(ExecutorService executor) {
-        var ref = new AtomicReference<Thread>();
+        class Box { static volatile Thread thread;}
+        var latch = new CountDownLatch(1);
         executor.submit(() -> {
-            ref.set(Thread.currentThread());
+            Box.thread = Thread.currentThread();
+            latch.countDown();
             LockSupport.park();
         });
-        Thread thread;
+        try {
-        while ((thread = ref.get()) == null) {
+            latch.await();
-            Thread.sleep(10);
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
         }
-        return thread;
+        return Box.thread;
     }
 
     /**
-     * Returns true if the file contains #<tid>
+     * Returns true if a Thread is present in a plain text thread dump.
      */
     private static boolean isPresent(Path file, Thread thread) throws Exception {
         String expect = "#" + thread.threadId();
@@ -235,13 +340,4 @@ class DumpThreads {
             return stream.skip(n).findFirst().orElseThrow();
         }
     }
-
-    /**
-     * Print the given file to standard output.
-     */
-    private static void cat(Path file) throws Exception {
-        try (Stream<String> stream = Files.lines(file)) {
-            stream.forEach(System.out::println);
-        }
-    }
 }

test/lib/jdk/test/lib/threaddump/ThreadDump.java

@@ -176,7 +176,6 @@ public final class ThreadDump {
                     .findAny();
         }
 
-
         /**
          * Helper method to recursively find a container with the given name.
          */