On Tue, 29 Oct 2024 16:41:31 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> Larry Cable has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> JDK-8342449: changed logic of attach loop to throw if target still not
>> ready when timed out and consolidated comments
>
> src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line
> 114:
>
>> 112: String.format("Unable to open socket file %s: "
>> +
>> 113: "target process %d doesn't respond within
>> %dms " +
>> 114: "or HotSpot VM not loaded", socket_path,
>> time_spend));
>
> Do we still need a pid argument after this format string?
> time_spent would read more easily than "spend" 8-) but we had have that for
> years.
generally I dont think its a good idea to modify exception msgs ...
> src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line
> 330:
>
>> 328: private static final long SIGQUIT = 1L << 2;
>> 329:
>> 330: private static boolean checkCatchesAndSendQuitTo(int pid, boolean
>> throwIfNotReady) throws AttachNotSupportedException, IOException {
>
> Looks like the throwIfNotReady param is not needed, can be removed.
I changed the logic so that now when the timeout loop times out the last
attempt to send the QUIT will throw if the target is not catching QUIT
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21688#discussion_r1821397298
PR Review Comment: https://git.openjdk.org/jdk/pull/21688#discussion_r1821400072
