On Thu, 27 Oct 2022 14:43:30 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose002a.java
>>  line 130:
>> 
>>> 128:                              instruction = pipe.readln();
>>> 129:                              if (instruction.equals("check_done")) {
>>> 130:                                  if (Utils.isAlive(test_thread)) {
>> 
>> We also have `jdk.test.lib.Utils` being referenced as `Utils` in the main 
>> test. Maybe `JDIUtils` would be better.
>> 
>> Also, `isAlive` is a bit generic sounding. It would be nice if the name 
>> conveyed the relationship to waiting after a dispose. Maybe 
>> `isAliveAfterVMDispose`. At least then the user knows there's something 
>> special about the "is alive" check being made.
>
> Actually `isAliveAfterVMDispose` conveys the wrong message. The idea is to 
> give the `VM.dispose()` a chance to happen. Maybe `isAlive` should just 
> better document why it waits.

Or you could use the longer name `nsk.share.jdi.Utils` to differentiate...

Is it always the case that we're calling this `isAlive()` function after a 
`VM.dispose()` call?
I'm not so sure that's the case... See L135-142 below. That looks like a simple 
query response
and not necessarily after a `VM.dispose()` call...

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

PR: https://git.openjdk.org/jdk/pull/10877

Reply via email to