On Mon, 28 Oct 2024 20:27:27 GMT, Larry Cable <[email protected]> wrote:
>> the implementation I originally provided does not in fact solve the issue!
>>
>> the attach protocol initiation "handshake" requires that the "attacher" (the
>> caller of this code) and the "attachee"(the target JVM to be "attached" to)
>> *must* share a "/tmp" (or access to the attachee's `cwd`) in common in
>> order to rendezvous on the "attach" socket (created in the /tmp or attachee
>> `cwd` filesystem).
>>
>> "attacher" and "attachee" JVM processes are guaranteed to share a common
>> directory/filesystem when thy occupy the same "mount namespace", this is the
>> environment in which "peers" exist, and the attach
>> handshake (initiated by the attacher) can use "/tmp" to establish the socket
>> connection with the attachee.
>>
>> with the advent of "containers" (implemented in Linux via various
>> namespaces, e.g.: pid & mount) another "attacher" and "attachee"
>> relationship exists, that of "attacher" (namespace ancestor) and "attachee"
>> (namespace descendant).
>>
>> in this environment it is possible (and highly probable) that the "attacher"
>> and the "attachee" do not share a directory in common.
>>
>> In this scenario the "attacher" must resort to handshaking with the attachee
>> via the /proc filesystem in order to access the "attachee's" directory from
>> the "attacher".
>>
>> In order to achieve this rendezvous, the "attachee" must occupy a
>> descendant, or same, (pid) namespace of, or as, the "attacher".
>>
>> since (pid) namespaces are hierarchical, a descendant process (in its own
>> descendent pid namespace) will also occupy all its ancestor (pid) namespaces
>> (between it and the 'root' or 'host' pid namespace) with a unique pid in
>> each of those "interstitial" (pid) namespace(s).
>>
>> thus the "attachee" directory is accessible, via an "ancestor's" (or peer's)
>> /proc filesystem using the pid of the "attachee" that is associated with it
>> in that (pid) namespace.
>>
>> thus an "ancestor" "attacher" can handshake with a descendant "attachee" in
>> this fashion.
>>
>> therefore an "attacher" has two choices when attempting to attach:
>>
>> - use the /proc/<pid>/root/tmp path to the "attachee's" /tmp (or its cwd)
>> - this works with both peers and descendants
>>
>> - use /tmp
>> - this only works if the "attacher" and "attachee" share a /tmp in common
>>
>> the obvious choice is to default to /proc/<pid>/root/tmp (or cwd) however
>> there is an issue with this; should the attachee have elevated privileges,
>> the attacher may not have r/w permission on the attachee's /proc/<pid>/root
>> (or cwd) path.
>>
>> I...
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21688#discussion_r1821191477
