> This test iterates an array of ThreadInfos in a few places (e.g. in the > method doCheck()), and needs to tolerate and ignore nulls, in case a thread > finishes and the test hits an NPE. > > There are other calls like "TM.getThreadInfo(tid).getLockName()" which might > often be risky, but if the threads are blocked as they are here, they can't > be terminating, so this usage is safe. > > > The test has additional problems when started in a virtual thread. > ThreadMXBean.getThreadInfo() methods only return a ThreadInfo for platform > threads. The test needs to avoid some checks if mainThread is virtual. > > In assertNoLock, it needs to not object to a thread holding a lock on a > VirtualThread object is not relevant. > Also the loop in doChecks which follows a chain of locks... This needs to > recognise that ForkJoinPool thead is not worth pursuing. It's not one of the > very narrow set of threads this test cares about. > > Despite these exclusions, the test does some reasonable verification work > when MainThread is virtual. This test historically cam in with a general > "JVM monitoring and management API" change, it is not testing a particular > fix. > > > There's a failure condition in doCheck() which will not make the test fail: > if it logs "TEST FAILED" in its final for loop, there is no failure. Make > the loop count the failures, and throw if there are any. > > Also, while looking into this... The variable names in some methods are > confusing. In checkBlockedObject(), let's use "threadName" rather than > "result" if we are finding a thread name, and let's not reuse the same result > variable for a lockName later in the method. > > The logs from this test are hard to read and verify, I find it better if the > lock objects OBJB and OBJC are of classes other than Object, so you get to > read, e.g.: > LockAThread blocked on Locks$ObjectB@4691fdfd > (ObjectB, not just Object).
Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - comment - Merge remote-tracking branch 'upstream/master' into 8306446_ThreadMXBean_Locks - feedback - Comment update - Update test/jdk/java/lang/management/ThreadMXBean/Locks.java Co-authored-by: Andrey Turbanov <turban...@gmail.com> - condition change - 8306446: java/lang/management/ThreadMXBean/Locks.java transient failures ------------- Changes: - all: https://git.openjdk.org/jdk/pull/14501/files - new: https://git.openjdk.org/jdk/pull/14501/files/02ba2a31..2767ed08 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14501&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14501&range=03-04 Stats: 125942 lines in 2425 files changed: 46444 ins; 67914 del; 11584 mod Patch: https://git.openjdk.org/jdk/pull/14501.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14501/head:pull/14501 PR: https://git.openjdk.org/jdk/pull/14501
