On Wed, 3 Dec 2025 07:22:51 GMT, Sergey Chernyshev <[email protected]> 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 possibility 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 t... > > Sergey Chernyshev has updated the pull request incrementally with one > additional commit since the last revision: > > addressed review comments This is a nice fix in general. Thank you for this work! I hope to complete my review tomorrow with one more pass. Question: How was this update tested? Do you have a jtreg test or you've done it manually? src/hotspot/os/bsd/os_bsd.cpp line 956: > 954: } > 955: #endif > 956: Nit: I'd suggest to get rid of empty lines which do not improve readability: 890, 903, 908, 917, 923, 930, 933, 941, 944 Also, the comment at line 931 is better to start with a capital letter: `// If the ..` . ------------- PR Review: https://git.openjdk.org/jdk/pull/25824#pullrequestreview-3533557192 PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583937603
