RFR: 8306446: java/lang/management/ThreadMXBean/Locks.java transient failures [v4]

[Chris Plummer]

On Wed, 26 Jul 2023 20:14:16 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> 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 at 4691fdfd
>> (ObjectB, not just Object).
>
> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
> 
>   feedback

Marked as reviewed by cjplummer (Reviewer).

-------------

PR Review: https://git.openjdk.org/jdk/pull/14501#pullrequestreview-1550425198