RFR: 8319589: Attach from root to a user java process not supported in Mac [v2]

Tue Sep 23 02:51:37 UTC 2025

On Wed, 18 Jun 2025 16:40:53 GMT, Sergey Chernyshev <schernyshev at openjdk.org> wrote:

>> Hi all,
>> 
>> I would like to propose a fix for JDK-8319589. This will allow jcmd and jps running as root to get the complete list of JVMs running by all users, and to attach from root to non-root JVMs. Previously, JDK-8197387 introduced the same approach on Linux.
>> 
>> This change affects macOS, that uses "secure" per-user temporary directories. It only affects JVMs running as root, the behavior in non-privileged JVMs remains unchanged.
>> 
>> Jcmd and jps rely on LocalVmManager to get the initial list of the local VMs. The LocalVmManager uses sun.jvmstat.PlatformSupport to get the list of temp directories, where it searches for user's PerfData directory such as "hsperfdata_<username\>". In macosx the temp directories are per-user, the temp path is returned by confstr(_CS_DARWIN_USER_TEMP_DIR). The per-user directories are mode 700 and so they are read-protected from non-privileged users and can be accessed by the owner and the root.
>> 
>> Both jps and jcmd (HotSpotAttachProvider) create MonitoredVm objects, that have PerfDataBuffer that performs attachment to the target. Only the attachable VMs are listed in jcmd output.
>> 
>> The proposed patch changes the list of directories returned by the PlatformSupport#getTemporaryDirectories() in VMs running as root. The list is later used in VirtualMachineImpl (jdk.attach). It changes also the way mmap_attach_shared() searches for hsperfdata_<username\>/<pid\> files to map the shared memory. Mmap_attach_shared() and VirtualMachineImpl (via PlatformSupport) list the content of /var/folders, where the temp directories are located, more specificly the temp directories are /var/folders/<BUCKET\>/<ENCODED_UUID_UID\>/T as hinted in [1]. The full list is returned by newly added PlatformSupportImpl#getTemporaryDirectories().
>> 
>> The attaching client's VirtualMachineImpl needs the target process's temp directory to find .java<pid\> and create .attach<pid\> files. It uses the list returned by PlatformSupportImpl#getTemporaryDirectories() and the ProcessHandle of the target process to search for user's PerfData directory, e.g. hsperfdata_<username\>, which is in the target process's temp directory, exactly where it expects to see the .java<pid\> in return on sending SIGQUIT to the target VM.
>> 
>> Mmap_attach_shared() traverses the /var/folders in get_user_tmp_dir() and looks for a hsperfdata_<username\> folder. If that folder is found in /var/folders/*/*/T, that means the temp folder corresponds to the <username\> and to the ...
>
> Sergey Chernyshev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Update src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
>    
>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>  - addressed review comments

This seems reasonable but I am far from expert in this area of macOS, nor in the Java side of this code. Serviceability need to take a good look at this.

I few minor nits below.

Thanks

src/hotspot/os/bsd/os_bsd.cpp line 893:

> 891: //
> 892: static const char VAR_FOLDERS[] = "/var/folders/";
> 893: int os::Bsd::get_user_tmp_dir_macos(const char* user, int vmid, char *output_path, int output_size /* = PATH_MAX */) {

Suggestion:

int os::Bsd::get_user_tmp_dir_macos(const char* user, int vmid, char* output_path, int output_size) {

src/hotspot/os/bsd/os_bsd.cpp line 912:

> 910:       // absolute path to the bucket
> 911:       char bucket[PATH_MAX];
> 912:       int b = snprintf(bucket, PATH_MAX, "%s%s/", VAR_FOLDERS, bucket_de->d_name);

Suggestion:

      int b = os::snprintf(bucket, PATH_MAX, "%s%s/", VAR_FOLDERS, bucket_de->d_name);

Just for clarity

src/hotspot/os/bsd/os_bsd.cpp line 936:

> 934:         //
> 935:         char perfdata_path[PATH_MAX];
> 936:         int p = snprintf(perfdata_path, PATH_MAX, "%s%s/T/%s_%s/", bucket, subbucket_de->d_name, PERFDATA_NAME, user);

Suggestion:

        int p = os::snprintf(perfdata_path, PATH_MAX, "%s%s/T/%s_%s/", bucket, subbucket_de->d_name, PERFDATA_NAME, user);

src/hotspot/os/bsd/os_bsd.cpp line 947:

> 945: 
> 946:           // the return value of snprintf is not checked for the second time
> 947:           return snprintf(output_path, output_size, "%s%s/T", bucket, subbucket_de->d_name);

Suggestion:

          return os::snprintf(output_path, output_size, "%s%s/T", bucket, subbucket_de->d_name);

src/hotspot/os/bsd/os_bsd.hpp line 68:

> 66: 
> 67: #ifdef __APPLE__
> 68:   static int get_user_tmp_dir_macos(const char* user, int vmid, char *output_buffer, int buffer_size /* = PATH_MAX */);

Suggestion:

  static int get_user_tmp_dir_macos(const char* user, int vmid, char* output_buffer, int buffer_size);

The buffer size is up to the caller.

src/hotspot/os/posix/perfMemory_posix.cpp line 1180:

> 1178:   int nspid = LINUX_ONLY(os::Linux::get_namespace_pid(vmid)) NOT_LINUX(-1);
> 1179:   const char* luser = NOT_MACOS(get_user_name(vmid, &nspid, CHECK))
> 1180:     MACOS_ONLY(get_user_name(os::Bsd::get_process_uid(vmid)));

Suggestion:

  const char* luser = NOT_MACOS(get_user_name(vmid, &nspid, CHECK))
                      MACOS_ONLY(get_user_name(os::Bsd::get_process_uid(vmid)));

src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line 31:

> 29: import com.sun.tools.attach.spi.AttachProvider;
> 30: 
> 31: import sun.jvmstat.PlatformSupport;

This is convenient but I'm not sure it is appropriate. Need the serviceability folk to comment.

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?

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

PR Review: https://git.openjdk.org/jdk/pull/25824#pullrequestreview-3255835341
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370839037
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370840143
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370841710
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370842039
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370844530
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370846681
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370851193
PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2370853656