On Mon, 20 Feb 2023 12:54:06 GMT, Kevin Walls <kev...@openjdk.org> wrote:
> Test update to make failures such as JDK-8076494 more informative.
>
> Waiting for a thread to change state: give more time (to distinguish a real
> deadlock from some other delay), and log the thread stackframes when the
> expected state change is not observed.
Approach is fine in principle but a few minor nits.
Thanks.
test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/thread/ThreadUtils.java line
232:
> 230:
> 231: // Most uses of waitThreadState usually succeed without retries.
> 232: // Sleep time increased to help avoid spurious failures.
"increased" doesn't mean anything outside this PR as you can't tell the value
has changed.
test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/thread/ThreadUtils.java line
239:
> 237: int retries = 0;
> 238: long ctime = System.currentTimeMillis();
> 239: long ctime2 = 0;
So ctime -> startTime; ctime2 -> elapsedTime ?
test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/thread/ThreadUtils.java line
242:
> 240: while (thread.getState() != state) {
> 241: if (retries++ > waitThreadStateRetries) {
> 242: // Failed to see desired state, give info to
> help diagnose the isuse.
This file is using an indent of 8. Looks odd to use 4 for the modified code
only. Maybe you could fix all the indents before integration?
test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/thread/ThreadUtils.java line
251:
> 249: }
> 250: try {
> 251: Thread.sleep(waitThreadStateSleepTime);
This file is using an indent of 8
-------------
Changes requested by dholmes (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12661
