On Mon, 6 May 2024 18:31:06 GMT, Larry Cable <d...@openjdk.org> wrote:
>> Sebastian Lövdahl has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Reworked attach logic > > On 5/6/24 10:35 AM, Sebastian Lövdahl wrote: >> >> I pushed an updated attempt at this now with d3e26a0 >> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/commit/d3e26a0c444e06ba9757fd528d72d83f56cd098b__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svPHZrq9ZQ$>. >> >> Local testing and |make test >> TEST="jtreg:test/hotspot/jtreg/containers"| + |make test >> TEST="jtreg:test/hotspot/jtreg/serviceability"| indicate that all the >> known use-cases work. >> >> Still eager to see what you come up with @larry-cable >> <https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svNMPdGFLg$>. >> >> |createAttachFile| could still be improved for example. And I would >> also be interested in looking into writing some test for the elevated >> privileges case. >> >> — >> Reply to this email directly, view it on GitHub >> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2096564990__;Iw!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svNUwHWtZA$>, >> >> or unsubscribe >> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67SGOTAXCKY2TO2OBDDZA65N5AVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJWGU3DIOJZGA__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svO2ymONGw$>. >> You are receiving this because you were mentioned.Message ID: >> ***@***.***> >> > > > /* > diff --git > a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java > b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java > index 81d4fd259ed..c148dbd61b7 100644 > --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java > +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java > @@ -34,6 +34,7 @@ > import java.nio.file.Path; > import java.nio.file.Paths; > import java.nio.file.Files; > +import java.util.Optional; > > import static java.nio.charset.StandardCharsets.UTF_8; > > @@ -46,8 +47,28 @@ public class VirtualMachineImpl extends > HotSpotVirtualMachine { > // location is the same for all processes, otherwise the tools > // will not be able to find all Hotspot processes. > // Any changes to this needs to be synchronized with HotSpot. > - private static final String tmpdir = "/tmp"; > + private static final Path TMPD... Thanks for all the deep thinking you're doing here @larry-cable, appreciated. And sorry for the delay in my response, I'll try to get more time devoted to this during the coming week. > I did some thinking on this issue over the weekend and came up with an > idea that *may* improve the probability of an attach succeeding in the > case that the target has elevated privileges and the jcmd is not in the > same mnt namespace as the target JVM. I haven't fully digested the patches you have provided yet, but one question this far. In these cases, is it not a requirement that `jcmd` is run as `root`? So even if the target process is run with elevated privileges, attaching would always work. Or is there some way to attach from host to container with a non-`root` user that I'm missing? One thing I would like to eventually achieve is to have [JDK-8226919](https://bugs.openjdk.org/browse/JDK-8226919) and this fix backported to the current LTS releases. Would it make more sense to fix [JDK-8327114](https://bugs.openjdk.org/browse/JDK-8327114) with as few changes as possible, and use that for backports, and do more extensive improvements in a separate follow-up that don't need backporting? ------------- PR Comment: https://git.openjdk.org/jdk/pull/19055#issuecomment-2106339738
