[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8306446: java/lang/management/ThreadMXBean/Locks.java transient failures [v6]
From:       Kevin Walls <kevinw () openjdk ! org>
Date:       2023-07-28 9:49:26
Message-ID: wLqqX_x0HhQWNDtJanEHtpXrA_t53UwlGr31iI5TN_I=.a39a61fa-c1cb-4d58-bb7a-79c0102a3eb8 () github ! com
[Download RAW message or body]

> 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 incrementally with one additional commit \
since the last revision:

  comment

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14501/files
  - new: https://git.openjdk.org/jdk/pull/14501/files/2767ed08..8107ee72

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14501&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14501&range=04-05

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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


[prev in list] [next in list] [prev in thread] [next in thread]