RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v2]

[Sebastian Lövdahl]

On Mon, 6 May 2024 18:31:06 GMT, Larry Cable <duke at 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...

I tested your patch @larry-cable, and, as far as I can tell, everything works. I ran through all the cases in https://github.com/openjdk/jdk/pull/19055#issuecomment-2090136676 once more to verify this. Thanks! Feel free to take a look.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19055#issuecomment-2123072183