On Wed, 11 Dec 2024 21:56:20 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.
>
> Alex Menkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> updated comment
The fix looks good modulo a couple of nits.
src/hotspot/share/services/attachListener.cpp line 670:
> 668: // for v1 also name/arguments should not exceed
> name_length_max/arg_length_max.
> 669: if (strlen(name()) > AttachOperation::name_length_max) {
> 670: log_error(attach)("Failed to read request: name is too long");
Nit: I'd suggest to be more specific: "Failed to read request: attach operation
name is too long"
src/hotspot/share/services/attachListener.hpp line 238:
> 236: virtual void complete(jint result, bufferedStream* result_stream) = 0;
> 237:
> 238: class ReplyWriter; //forward declaration
Nit: The comment should start with a space.
-------------
Marked as reviewed by sspitsyn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22223#pullrequestreview-2502115945
PR Review Comment: https://git.openjdk.org/jdk/pull/22223#discussion_r1883819157
PR Review Comment: https://git.openjdk.org/jdk/pull/22223#discussion_r1883821145
