From 3bf5022bc6b8bbc544502b3fc100c6debdb1b2c7 Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Wed, 25 Mar 2026 07:40:26 +0000 Subject: [PATCH] 8380641: Thread dump parsing and test improvements 8378946: threadDump.schema.json syntax error, missing comma after owner Reviewed-by: amenkov, sspitsyn --- .../doc-files/threadDump.schema.json | 2 +- .../HotSpotDiagnosticMXBean/DumpThreads.java | 84 +++++--- .../security/provider/acvp/ML_DSA_Test.java | 10 +- test/lib/jdk/test/lib/json/JSONValue.java | 94 +++++++-- .../jdk/test/lib/threaddump/ThreadDump.java | 193 +++++++++--------- 5 files changed, 240 insertions(+), 143 deletions(-) diff --git a/src/jdk.management/share/classes/com/sun/management/doc-files/threadDump.schema.json b/src/jdk.management/share/classes/com/sun/management/doc-files/threadDump.schema.json index bf52bb3915d..1da3e3941ef 100644 --- a/src/jdk.management/share/classes/com/sun/management/doc-files/threadDump.schema.json +++ b/src/jdk.management/share/classes/com/sun/management/doc-files/threadDump.schema.json @@ -81,7 +81,7 @@ "owner": { "type": "string", "description": "The thread identifier of the owner when the parkBlocker is an AbstractOwnableSynchronizer." - } + }, "required": [ "object" ] diff --git a/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java b/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java index 77020491c29..3878513d3f2 100644 --- a/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java +++ b/test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2026, 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 @@ -49,6 +49,7 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.Path; import java.time.ZonedDateTime; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -238,6 +239,7 @@ class DumpThreads { void testBlockedThread(ThreadFactory factory, boolean pinned) throws Exception { var lock = new Object(); + String lockAsString = Objects.toIdentityString(lock); var started = new CountDownLatch(1); Thread thread = factory.newThread(() -> { @@ -258,9 +260,7 @@ class DumpThreads { thread.start(); started.await(); await(thread, Thread.State.BLOCKED); - long tid = thread.threadId(); - String lockAsString = Objects.toIdentityString(lock); // thread dump in plain text should include thread List lines = dumpThreadsToPlainText(); @@ -308,6 +308,7 @@ class DumpThreads { void testWaitingThread(ThreadFactory factory, boolean pinned) throws Exception { var lock = new Object(); + String lockAsString = Objects.toIdentityString(lock); var started = new CountDownLatch(1); Thread thread = factory.newThread(() -> { @@ -331,9 +332,7 @@ class DumpThreads { thread.start(); started.await(); await(thread, Thread.State.WAITING); - long tid = thread.threadId(); - String lockAsString = Objects.toIdentityString(lock); // thread dump in plain text should include thread List lines = dumpThreadsToPlainText(); @@ -417,7 +416,6 @@ class DumpThreads { thread.start(); started.await(); await(thread, Thread.State.WAITING); - long tid = thread.threadId(); // thread dump in plain text should include thread @@ -460,7 +458,7 @@ class DumpThreads { } /** - * Test thread dump with a thread owning a monitor. + * Test thread dump with a thread owning monitors. */ @ParameterizedTest @MethodSource("threadFactories") @@ -475,19 +473,26 @@ class DumpThreads { } void testThreadOwnsMonitor(ThreadFactory factory, boolean pinned) throws Exception { - var lock = new Object(); - var started = new CountDownLatch(1); + var lock1 = new Object(); + var lock2 = new Object(); + var lock3 = new Object(); + String lock1AsString = Objects.toIdentityString(lock1); + String lock2AsString = Objects.toIdentityString(lock2); + String lock3AsString = Objects.toIdentityString(lock3); + var started = new CountDownLatch(1); Thread thread = factory.newThread(() -> { - synchronized (lock) { - if (pinned) { - VThreadPinner.runPinned(() -> { + synchronized (lock1) { + synchronized (lock2) { + if (pinned) { + VThreadPinner.runPinned(() -> { + started.countDown(); + lockAndRun(lock3, LockSupport::park); + }); + } else { started.countDown(); - LockSupport.park(); - }); - } else { - started.countDown(); - LockSupport.park(); + lockAndRun(lock3, LockSupport::park); + } } } }); @@ -497,16 +502,16 @@ class DumpThreads { thread.start(); started.await(); await(thread, Thread.State.WAITING); - long tid = thread.threadId(); - String lockAsString = Objects.toIdentityString(lock); // thread dump in plain text should include thread List lines = dumpThreadsToPlainText(); ThreadFields fields = findThread(tid, lines); assertNotNull(fields, "thread not found"); assertEquals("WAITING", fields.state()); - assertTrue(contains(lines, "- locked <" + lockAsString)); + assertTrue(contains(lines, "- locked <" + lock1AsString)); + assertTrue(contains(lines, "- locked <" + lock2AsString)); + assertTrue(contains(lines, "- locked <" + lock3AsString)); // thread dump in JSON format should include thread in root container ThreadDump threadDump = dumpThreadsToJson(); @@ -516,18 +521,47 @@ class DumpThreads { assertNotNull(ti, "thread not found"); assertEquals(ti.isVirtual(), thread.isVirtual()); - // the lock should be in the ownedMonitors array - Set ownedMonitors = ti.ownedMonitors().values() + // depth -> list of locks + Map> ownedMonitors = ti.ownedMonitors(); + + // lock -> list of depths + Map> monitorDepths = ownedMonitors.entrySet() .stream() - .flatMap(List::stream) - .collect(Collectors.toSet()); - assertTrue(ownedMonitors.contains(lockAsString), lockAsString + " not found"); + .flatMap(e -> e.getValue() + .stream() + .map(monitor -> Map.entry(monitor, e.getKey()))) + .collect(Collectors.groupingBy( + Map.Entry::getKey, + Collectors.mapping(Map.Entry::getValue, Collectors.toList()) + )); + + // each lock should be owned + List lock1Depths = monitorDepths.getOrDefault(lock1AsString, List.of()); + List lock2Depths = monitorDepths.getOrDefault(lock2AsString, List.of()); + List lock3Depths = monitorDepths.getOrDefault(lock3AsString, List.of()); + assertEquals(1, lock1Depths.size()); + assertEquals(1, lock2Depths.size()); + assertEquals(1, lock3Depths.size()); + + // lock1 and lock2 owned at the same depth, lock3 is the innermost + int depth1 = lock1Depths.get(0); + int depth2 = lock2Depths.get(0); + int depth3 = lock3Depths.get(0); + assertEquals(depth1, depth2); + assertTrue(depth3 < depth1); + } finally { LockSupport.unpark(thread); thread.join(); } } + private void lockAndRun(Object lock, Runnable action) { + synchronized (lock) { + action.run(); + } + } + /** * Test mounted virtual thread. */ diff --git a/test/jdk/sun/security/provider/acvp/ML_DSA_Test.java b/test/jdk/sun/security/provider/acvp/ML_DSA_Test.java index ac56642b8d7..f76f3d8b9a8 100644 --- a/test/jdk/sun/security/provider/acvp/ML_DSA_Test.java +++ b/test/jdk/sun/security/provider/acvp/ML_DSA_Test.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2026, 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 @@ -88,13 +88,13 @@ public class ML_DSA_Test { for (var t : kat.get("testGroups").asArray()) { var pname = t.get("parameterSet").asString(); System.out.println(">> " + pname + " sign"); - var det = Boolean.parseBoolean(t.get("deterministic").asString()); + var det = t.get("deterministic").asBoolean(); if (t.get("signatureInterface").asString().equals("internal")) { ML_DSA_Impls.version = ML_DSA_Impls.Version.DRAFT; } else { ML_DSA_Impls.version = ML_DSA_Impls.Version.FINAL; } - if (t.get("externalMu").asString().equals("true")) { + if (t.get("externalMu").asBoolean()) { continue; // Not supported } for (var c : t.get("tests").asArray()) { @@ -139,7 +139,7 @@ public class ML_DSA_Test { ML_DSA_Impls.version = ML_DSA_Impls.Version.FINAL; } - if (t.get("externalMu").asString().equals("true")) { + if (t.get("externalMu").asBoolean()) { continue; // Not supported } @@ -157,7 +157,7 @@ public class ML_DSA_Test { public byte[] getEncoded() { return toByteArray(c.get("pk").asString()); } }; // Only ML-DSA sigVer has negative tests - var expected = Boolean.parseBoolean(c.get("testPassed").asString()); + var expected = c.get("testPassed").asBoolean(); var actual = true; try { s.initVerify(pk); diff --git a/test/lib/jdk/test/lib/json/JSONValue.java b/test/lib/jdk/test/lib/json/JSONValue.java index f89d13b3bba..72ed2fd917c 100644 --- a/test/lib/jdk/test/lib/json/JSONValue.java +++ b/test/lib/jdk/test/lib/json/JSONValue.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2026, 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 @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; public interface JSONValue { @@ -88,9 +89,6 @@ public interface JSONValue { @Override public String toString() { - if (value == null) { - return "null"; - } var builder = new StringBuilder(); builder.append("\""); @@ -172,6 +170,56 @@ public interface JSONValue { public Iterator iterator() { return values.iterator(); } + + @Override + public List elements() { + return List.copyOf(values); + } + } + + public final class JSONBoolean implements JSONValue { + private static JSONBoolean TRUE = new JSONBoolean(true); + private static JSONBoolean FALSE = new JSONBoolean(false); + + private final boolean value; + + private JSONBoolean(boolean value) { + this.value = value; + } + + @Override + public boolean asBoolean() { + return value; + } + + @Override + public String toString() { + return String.valueOf(value); + } + + public static JSONBoolean of(boolean value) { + return value ? TRUE : FALSE; + } + } + + public final class JSONNull implements JSONValue { + private static JSONNull NULL = new JSONNull(); + + private JSONNull() {} + + @Override + public Optional valueOrNull() { + return Optional.empty(); + } + + @Override + public String toString() { + return "null"; + } + + public static JSONNull of() { + return NULL; + } } class JSONParser { @@ -181,8 +229,8 @@ public interface JSONValue { JSONParser() { } - private IllegalStateException failure(String message) { - return new IllegalStateException(String.format("[%d]: %s : %s", pos, message, input)); + private IllegalArgumentException failure(String message) { + return new IllegalArgumentException(String.format("[%d]: %s : %s", pos, message, input)); } private char current() { @@ -220,13 +268,13 @@ public interface JSONValue { } } - private JSONString parseBoolean() { + private JSONBoolean parseBoolean() { if (current() == 't') { expect('r'); expect('u'); expect('e'); advance(); - return new JSONString("true"); + return JSONBoolean.of(true); } if (current() == 'f') { @@ -235,7 +283,7 @@ public interface JSONValue { expect('s'); expect('e'); advance(); - return new JSONString("false"); + return JSONBoolean.of(false); } throw failure("a boolean can only be 'true' or 'false'"); } @@ -400,12 +448,12 @@ public interface JSONValue { return new JSONArray(list); } - public JSONString parseNull() { + public JSONNull parseNull() { expect('u'); expect('l'); expect('l'); advance(); - return new JSONString(null); + return JSONNull.of(); } public JSONObject parseObject() { @@ -531,22 +579,38 @@ public interface JSONValue { } default int size() { - throw new IllegalStateException("Size operation unsupported"); + throw new UnsupportedOperationException("Size operation unsupported"); + } + + default List elements() { + throw new UnsupportedOperationException("Unsupported conversion to array"); } default String asString() { - throw new IllegalStateException("Unsupported conversion to String"); + throw new UnsupportedOperationException("Unsupported conversion to String"); } default JSONArray asArray() { - throw new IllegalStateException("Unsupported conversion to array"); + throw new UnsupportedOperationException("Unsupported conversion to array"); } default JSONObject asObject() { - throw new IllegalStateException("Unsupported conversion to object"); + throw new UnsupportedOperationException("Unsupported conversion to object"); + } + + default boolean asBoolean() { + throw new UnsupportedOperationException("Unsupported conversion to boolean"); } default JSONValue get(String field) { return asObject().get(field); } + + default Optional getOrAbsent(String field) { + return Optional.ofNullable(get(field)); + } + + default Optional valueOrNull() { + return Optional.of(this); + } } diff --git a/test/lib/jdk/test/lib/threaddump/ThreadDump.java b/test/lib/jdk/test/lib/threaddump/ThreadDump.java index ca728e625fc..972d46675f4 100644 --- a/test/lib/jdk/test/lib/threaddump/ThreadDump.java +++ b/test/lib/jdk/test/lib/threaddump/ThreadDump.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2026, 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 @@ -32,13 +32,17 @@ import java.util.Map; import java.util.Optional; import java.util.OptionalLong; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import jdk.test.lib.json.JSONValue; /** * Represents a thread dump that is obtained by parsing JSON text. A thread dump in JSON * format is generated with the {@code com.sun.management.HotSpotDiagnosticMXBean} API or - * using {@code jcmd Thread.dump_to_file -format=json }. + * using {@code jcmd Thread.dump_to_file -format=json }. The thread dump + * format is documented in {@code + * src/jdk.management/share/classes/com/sun/management/doc-files/threadDump.schema.json}. * *

The following is an example thread dump that is parsed by this class. Many of the * objects are collapsed to reduce the size. @@ -127,6 +131,20 @@ public final class ThreadDump { this.threadDumpObj = threadDumpObj; } + /** + * Assert that a JSONValue is a JSONString and parse the string as an int. + */ + private static int parseStringAsInt(JSONValue valueObj) { + return Integer.parseInt(valueObj.asString()); + } + + /** + * Assert that a JSONValue is a JSONString and parse the string as a long. + */ + private static long parseStringAsLong(JSONValue valueObj) { + return Long.parseLong(valueObj.asString()); + } + /** * Represents an element in the threadDump/threadContainers array. */ @@ -149,14 +167,6 @@ public final class ThreadDump { children.add(container); } - /** - * Returns the value of a property of this thread container, as a string. - */ - private String getStringProperty(String propertyName) { - JSONValue value = containerObj.get(propertyName); - return (value != null) ? value.asString() : null; - } - /** * Returns the thread container name. */ @@ -168,10 +178,10 @@ public final class ThreadDump { * Return the thread identifier of the owner or empty OptionalLong if not owned. */ public OptionalLong owner() { - String owner = getStringProperty("owner"); - return (owner != null) - ? OptionalLong.of(Long.parseLong(owner)) - : OptionalLong.empty(); + return containerObj.get("owner") // string or null + .valueOrNull() + .map(v -> OptionalLong.of(parseStringAsLong(v))) + .orElse(OptionalLong.empty()); } /** @@ -192,12 +202,10 @@ public final class ThreadDump { * Returns a stream of {@code ThreadInfo} objects for the threads in this container. */ public Stream threads() { - JSONValue.JSONArray threadsObj = containerObj.get("threads").asArray(); - Set threadInfos = new HashSet<>(); - for (JSONValue threadObj : threadsObj) { - threadInfos.add(new ThreadInfo(threadObj)); - } - return threadInfos.stream(); + return containerObj.get("threads") + .elements() + .stream() + .map(ThreadInfo::new); } /** @@ -237,29 +245,10 @@ public final class ThreadDump { private final JSONValue threadObj; ThreadInfo(JSONValue threadObj) { - this.tid = Long.parseLong(threadObj.get("tid").asString()); + this.tid = parseStringAsLong(threadObj.get("tid")); this.threadObj = threadObj; } - /** - * Returns the value of a property of this thread object, as a string. - */ - private String getStringProperty(String propertyName) { - JSONValue value = threadObj.get(propertyName); - return (value != null) ? value.asString() : null; - } - - /** - * Returns the value of a property of an object in this thread object, as a string. - */ - private String getStringProperty(String objectName, String propertyName) { - if (threadObj.get(objectName) instanceof JSONValue.JSONObject obj - && obj.get(propertyName) instanceof JSONValue value) { - return value.asString(); - } - return null; - } - /** * Returns the thread identifier. */ @@ -271,83 +260,92 @@ public final class ThreadDump { * Returns the thread name. */ public String name() { - return getStringProperty("name"); + return threadObj.get("name").asString(); } /** * Returns the thread state. */ public String state() { - return getStringProperty("state"); + return threadObj.get("state").asString(); } /** * Returns true if virtual thread. */ public boolean isVirtual() { - String s = getStringProperty("virtual"); - return (s != null) ? Boolean.parseBoolean(s) : false; + return threadObj.getOrAbsent("virtual") + .map(JSONValue::asBoolean) + .orElse(false); } /** - * Returns the thread's parkBlocker. + * Returns the thread's parkBlocker or null. */ public String parkBlocker() { - return getStringProperty("parkBlocker", "object"); + return threadObj.getOrAbsent("parkBlocker") + .map(v -> v.get("object").asString()) + .orElse(null); } /** * Returns the owner of the parkBlocker if the parkBlocker is an AbstractOwnableSynchronizer. */ public OptionalLong parkBlockerOwner() { - String owner = getStringProperty("parkBlocker", "owner"); - return (owner != null) - ? OptionalLong.of(Long.parseLong(owner)) - : OptionalLong.empty(); + return threadObj.getOrAbsent("parkBlocker") + .map(v -> OptionalLong.of(parseStringAsLong(v.get("owner")))) + .orElse(OptionalLong.empty()); } /** - * Returns the object that the thread is blocked entering its monitor. + * Returns the object that the thread is blocked entering its monitor or null. */ public String blockedOn() { - return getStringProperty("blockedOn"); + return threadObj.getOrAbsent("blockedOn") + .map(JSONValue::asString) + .orElse(null); } /** - * Return the object that is the therad is waiting on with Object.wait. + * Return the object that is the thread is waiting on with Object.wait or null. */ public String waitingOn() { - return getStringProperty("waitingOn"); + return threadObj.getOrAbsent("waitingOn") + .map(JSONValue::asString) + .orElse(null); } /** * Returns the thread stack. */ public Stream stack() { - JSONValue.JSONArray stackObj = threadObj.get("stack").asArray(); - List stack = new ArrayList<>(); - for (JSONValue steObject : stackObj) { - stack.add(steObject.asString()); - } - return stack.stream(); + return threadObj.get("stack") + .elements() + .stream() + .map(JSONValue::asString); } /** * Return a map of monitors owned. */ public Map> ownedMonitors() { - Map> ownedMonitors = new HashMap<>(); - JSONValue monitorsOwnedObj = threadObj.get("monitorsOwned"); - if (monitorsOwnedObj != null) { - for (JSONValue obj : monitorsOwnedObj.asArray()) { - int depth = Integer.parseInt(obj.get("depth").asString()); - for (JSONValue lock : obj.get("locks").asArray()) { - ownedMonitors.computeIfAbsent(depth, _ -> new ArrayList<>()) - .add(lock.asString()); - } - } - } - return ownedMonitors; + Map> result = new HashMap<>(); + threadObj.getOrAbsent("monitorsOwned") + .map(JSONValue::elements) + .orElse(List.of()) + .forEach(e -> { + int depth = parseStringAsInt(e.get("depth")); + List locks = e.get("locks") + .elements() + .stream() + .map(v -> v.valueOrNull() // string or null + .map(JSONValue::asString) + .orElse(null)) + .toList(); + result.computeIfAbsent(depth, _ -> new ArrayList<>()).addAll(locks); + }); + + return result; } /** @@ -355,10 +353,9 @@ public final class ThreadDump { * its carrier. */ public OptionalLong carrier() { - String s = getStringProperty("carrier"); - return (s != null) - ? OptionalLong.of(Long.parseLong(s)) - : OptionalLong.empty(); + return threadObj.getOrAbsent("carrier") + .map(s -> OptionalLong.of(parseStringAsLong(s))) + .orElse(OptionalLong.empty()); } @Override @@ -388,33 +385,25 @@ public final class ThreadDump { } } - /** - * Returns the value of a property of this thread dump, as a string. - */ - private String getStringProperty(String propertyName) { - JSONValue value = threadDumpObj.get(propertyName); - return (value != null) ? value.asString() : null; - } - /** * Returns the value of threadDump/processId. */ public long processId() { - return Long.parseLong(getStringProperty("processId")); + return parseStringAsLong(threadDumpObj.get("processId")); } /** * Returns the value of threadDump/time. */ public String time() { - return getStringProperty("time"); + return threadDumpObj.get("time").asString(); } /** * Returns the value of threadDump/runtimeVersion. */ public String runtimeVersion() { - return getStringProperty("runtimeVersion"); + return threadDumpObj.get("runtimeVersion").asString(); } /** @@ -449,24 +438,31 @@ public final class ThreadDump { JSONValue threadDumpObj = JSONValue.parse(json).get("threadDump"); // threadContainers array, preserve insertion order (parents are added before children) - Map containerObjs = new LinkedHashMap<>(); - JSONValue threadContainersObj = threadDumpObj.get("threadContainers"); - for (JSONValue containerObj : threadContainersObj.asArray()) { - String name = containerObj.get("container").asString(); - containerObjs.put(name, containerObj); - } + Map containerObjs = threadDumpObj.get("threadContainers") + .elements() + .stream() + .collect(Collectors.toMap( + c -> c.get("container").asString(), + Function.identity(), + (a, b) -> { throw new RuntimeException("Duplicate container"); }, + LinkedHashMap::new + )); // find root and create tree of thread containers ThreadContainer root = null; Map map = new HashMap<>(); for (String name : containerObjs.keySet()) { JSONValue containerObj = containerObjs.get(name); - String parentName = containerObj.get("parent").asString(); - if (parentName == null) { + JSONValue parentObj = containerObj.get("parent"); + if (parentObj instanceof JSONValue.JSONNull) { + if (root != null) { + throw new RuntimeException("More than one root container"); + } root = new ThreadContainer(name, null, containerObj); map.put(name, root); } else { - var parent = map.get(parentName); + String parentName = parentObj.asString(); + ThreadContainer parent = map.get(parentName); if (parent == null) { throw new RuntimeException("Thread container " + name + " found before " + parentName); } @@ -475,7 +471,10 @@ public final class ThreadDump { map.put(name, container); } } + if (root == null) { + throw new RuntimeException("No root container"); + } return new ThreadDump(root, map, threadDumpObj); } -} \ No newline at end of file +}