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

[Kevin Walls]

On Wed, 21 Jun 2023 16:56:08 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:
> 
>   Comment update

Thanks Alan, yes-
All the TEST really knows is the couple of threads it creates, and the lock objects it creates.  When it says "assert no lock held", it means to assert that none of the locks that it created and cares about are held.

When MainThread is Virtual, as started by jtreg, the test asserts that no locks it cares about are held by MainThread.  When there is an additional locked object (a VirtualThread), that surprises the test. Hence the filter on getLockName() in assertNoLock.

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

PR Comment: https://git.openjdk.org/jdk/pull/14501#issuecomment-1602473395