From 2e9eff56418273e85accc43dcef533995c6be8bf Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Tue, 6 Jun 2023 06:01:38 +0000 Subject: [PATCH] 8309406: Change jdk.trackAllThreads to default to true Reviewed-by: rpressler, mchung, cjplummer --- .../internal/vm/SharedThreadContainer.java | 16 +- .../jdk/internal/vm/ThreadContainers.java | 50 +-- .../classes/jdk/internal/vm/ThreadDumper.java | 6 +- .../dcmd/thread/ThreadDumpToFileTest.java | 19 +- .../HotSpotDiagnosticMXBean/DumpThreads.java | 292 ++++++++++++------ .../jdk/test/lib/threaddump/ThreadDump.java | 1 - 6 files changed, 243 insertions(+), 141 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/vm/SharedThreadContainer.java b/src/java.base/share/classes/jdk/internal/vm/SharedThreadContainer.java index 2a454432c61..586f6a0b2ba 100644 --- a/src/java.base/share/classes/jdk/internal/vm/SharedThreadContainer.java +++ b/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 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 threads() { // live platform threads in this container diff --git a/src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java b/src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java index 75b23dd6e2f..bb9cd4513d9 100644 --- a/src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java +++ b/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 - * monitoring of threads. + * This class consists exclusively of static methods to support groupings 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> CONTAINER_REGISTRY = ConcurrentHashMap.newKeySet(); private static final ReferenceQueue 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 - * container plus some (or all) virtual threads that are started directly - * with the Thread API. + * Root container that "contains" all platform threads not started in a container. + * It may include all virtual threads started directly 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 threads() { - // virtual threads in this container that are those blocked on I/O. - Stream blockedVirtualThreads = Poller.blockedThreads() - .filter(t -> t.isVirtual() - && JLA.threadContainer(t) == this); - return Stream.concat(platformThreads(), blockedVirtualThreads); + return platformThreads(); } } } diff --git a/src/java.base/share/classes/jdk/internal/vm/ThreadDumper.java b/src/java.base/share/classes/jdk/internal/vm/ThreadDumper.java index a8c34983e88..53a3e3694f1 100644 --- a/src/java.base/share/classes/jdk/internal/vm/ThreadDumper.java +++ b/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) { diff --git a/test/hotspot/jtreg/serviceability/dcmd/thread/ThreadDumpToFileTest.java b/test/hotspot/jtreg/serviceability/dcmd/thread/ThreadDumpToFileTest.java index b2386adb617..f6002a32968 100644 --- a/test/hotspot/jtreg/serviceability/dcmd/thread/ThreadDumpToFileTest.java +++ b/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 stream = Files.lines(file)) { return stream.anyMatch(line -> line.indexOf(text) >= 0); diff --git a/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java b/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java index c31d27fa9bd..a4072f2f7ff 100644 --- a/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java +++ b/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; - static { + private static boolean trackAllThreads; + + @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 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 { - var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class); - Path file = genOutputPath("txt"); - try (var executor = Executors.newVirtualThreadPerTaskExecutor()) { - 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); - } + void testRootContainerPlainTextFormat() throws Exception { + Thread vthread = Thread.ofVirtual().start(LockSupport::park); + try { + testDumpThreadsPlainText(vthread, trackAllThreads); } 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 { - var mbean = ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class); - Path file = genOutputPath("json"); - try (var executor = Executors.newVirtualThreadPerTaskExecutor()) { - 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); - } + void testRootContainerJsonFormat() throws Exception { + Thread vthread = Thread.ofVirtual().start(LockSupport::park); + try { + testDumpThreadsJson(null, vthread, trackAllThreads); } 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 { - var ref = new AtomicReference(); + private static Thread forkParker(ExecutorService executor) { + 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; - while ((thread = ref.get()) == null) { - Thread.sleep(10); + try { + latch.await(); + } catch (InterruptedException e) { + throw new RuntimeException(e); } - return thread; + return Box.thread; } /** - * Returns true if the file contains # + * 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 stream = Files.lines(file)) { - stream.forEach(System.out::println); - } - } } diff --git a/test/lib/jdk/test/lib/threaddump/ThreadDump.java b/test/lib/jdk/test/lib/threaddump/ThreadDump.java index 3d288dda574..5e4f6ebc10f 100644 --- a/test/lib/jdk/test/lib/threaddump/ThreadDump.java +++ b/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. */