On Fri, 22 Mar 2024 20:31:01 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> feedback
>
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java
> line 116:
>
>> 114:
>> 115: boolean isConnected = true;
>> 116: boolean eventsReceived = false;
>
> Nit: It'd better to rename this local to `allEventsReceived`.
I tried to make minimal changes in the tests, so kept variable names to avoid
unnecessary changes
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java
> line 134:
>
>> 132: while (eventIterator.hasNext()) {
>> 133: Event event = eventIterator.nextEvent();
>> 134: // log.display("\nEvent received:\n " +
>> event);
>
> Nit: Would it make sense to uncomment this log message? How many events are
> normally printed?
The only event received are ClassPrepareEvent (requested by test) and
VMDeathEvent (when debuggee exits)
Both types are logged below
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java
> line 277:
>
>> 275: if (!eventsReceivedLatch.await(eventTimeout,
>> TimeUnit.MILLISECONDS)) {
>> 276: log.complain("FAILURE 20: Timeout waiting for all
>> events was exceeded");
>> 277: testFailed = true;
>
> As I understand the call to `eventHandler.interrupt()` is not needed now
> because now the `eventHandler` is expected to finish by itself when one of
> the events is received: `VMDeathEvent` or `VMDisconnectEvent`.
right. eventHandler exits when debuggee exits
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536200016
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536205663
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536207978
