On Tue, 23 Sep 2025 02:43:45 GMT, David Holmes <[email protected]> wrote:

>> Sergey Chernyshev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Apply suggestions from code review
>>   
>>   Co-authored-by: David Holmes 
>> <[email protected]>
>
> src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line 
> 49:
> 
>> 47:     // the latter can be changed by the user.
>> 48:     // Any changes to this needs to be synchronized with HotSpot.
>> 49:     private static final String tmpdir;
> 
> So we can't cache this any more?

Thanks David for pointing this out. A different instance with a different PID 
may have a different temp path, so the path should be specific to the instance. 
Since we don't need the path outside the constructor, I think we don't need to 
cache it. On the other hand I see that `getTempDirFromPid` is called twice, 
once for `.java_pid` and another time in `createAttachFile` when `.java_pid` 
doesn't exist. Would you suggest storing the path in a local variable in the 
constructor and then concatenating it with attach file name, or would a 
non-static member variable be more suitable for this?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2372940682

Reply via email to