On Fri, 22 Mar 2024 19:26:33 GMT, Alex Menkov <amen...@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
