On Tue, 19 Nov 2024 00:04:16 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> The fix updates Linux (and server-side of macosx) implementation to support > Attach API v2 (shared code and Windows implementation were introduced by > #20782) > > Testing: tier1,tier2,tier3,tier4,hs-tier5-svc > manually tested backward compatibility (old tools can attach to current VMs, > current tools can attach to older VMs) with jdk21 and jdk8. Looks good, thanks, a couple of other notes here in case it helps anyone. AttachOperation::write_reply(ReplyWriter * writer, jint result, const char* message, int message_len) is this only called by the write_reply method that passes message_len as strlen(stream) ? The new check for len<0 might not really be useful but does no harm. The existing "char msg[32]" is a buffer used to write the numeric jint result, not the message as the name might suggest. HotSpotVirtualMachine.java already has writeCommand() that handles VERSION_2, so the Linux VirtualMachineImpl can just call it. Also writeString is already there in HotSpotVirtualMachine, so removal from Linux is a good cleanup. I'm sure we all want to find time for the future conversation about actually streaming the output rather than passing a single buffer around. 8-) ------------- Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22223#pullrequestreview-2491720772
