RFR: 8302870: More information needed from failures in vmTestbase ThreadUtils.waitThreadState

[David Holmes]

On Mon, 20 Feb 2023 12:54:06 GMT, Kevin Walls <kevinw at 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