RFR: JDK-8328303: 3 JDI tests timed out with UT enabled [v2]

[Daniel D. Daugherty]

On Fri, 22 Mar 2024 19:26:33 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> The change fixes 3 nsk JDI tests.
>> Root cause in all 3 tests is the same - the tests requests JDI event with SUSPEND_ALL policy, but event handler thread stops handle incoming event and this causes debuggee to hang (suspended by JDI event).
>> 
>> All 3 tests are updated to exit event handler thread after getting VMDeathEvent or VMDisconnectEvent (and resume debuggee after any other events).
>> ClassPrepareEvent tests need to wait some time to allow handle all expected events before terminate the debuggee. The logic was implemented by using CountDownLatch.
>> 
>> All tests are passed with "--test-repeat 20"
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   feedback

Thumbs up. I only have nit comments.

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java line 117:

> 115:                     boolean isConnected = true;
> 116:                     boolean eventsReceived = false;
> 117:                     // handle events until debugee is disconnected

Nit typo: s/debugee/debuggee/

But a lot of the NSK tests have this typo...

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java line 203:

> 201:                                               }
> 202: 
> 203:                                               // Check that all expected ClassPrepareEvent are received

nit typo: s/ClassPrepareEvent/ClassPrepareEvent(s)/

nit typo: Since this comment starts with a capital, it should have a period at the end.

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java line 207:

> 205:                                                   eventsReceived = true;
> 206:                                                   for (int i = 0; i < checkedTypes.length; i++) {
> 207:                                                       if (checkedTypes[i][2] == "0") {

This if-statement could use a comment to explain the logic.

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java line 138:

> 136:                     boolean isConnected = true;
> 137:                     boolean eventsReceived = false;
> 138:                     // handle events until debugee is disconnected

Nit typo: s/debugee/debuggee/

But a lot of the NSK tests have this typo...

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java line 233:

> 231:                                           }
> 232: 
> 233:                                           // Check that all expected ClassPrepareEvent are received

nit typo: s/ClassPrepareEvent/ClassPrepareEvent(s)/

nit typo: Since this comment starts with a capital, it should have a period at the end.

test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java line 237:

> 235:                                               eventsReceived = true;
> 236:                                               for (int i = 0; i < checkedThreads.length; i++) {
> 237:                                                   if (checkedThreads[i][2] == "0") {

This if-statement could use a comment to explain the logic.

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18442#pullrequestreview-1955733734
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536172906
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536176808
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536178687
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536181360
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536183605
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536184067