Repository navigation
Use libproc instead of lsof on Darwin to detect live gcroots #16439
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| synopsis: Nix now finds Darwin runtime garbage collector roots without `lsof` | ||
| issues: [3011, 13990] | ||
| --- | ||
|
|
||
| On Darwin, the garbage collector now uses `libproc` to retain store paths | ||
| referenced by running processes through their working directories, open files, | ||
| mapped regions, or environment variables. This avoids the slow `lsof` fallback | ||
| and adds environment-variable roots, which `lsof` could not discover. | ||
|
|
||
| macOS does not expose the environment variables of entitled binaries while | ||
| System Integrity Protection is enabled, so those roots remain unavailable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| #include "../local-gc-private.hh" | ||
|
|
||
| #include "nix/util/error.hh" | ||
| #include "nix/util/fmt.hh" | ||
| #include "nix/util/signals.hh" | ||
|
|
||
| #include <libproc.h> | ||
| #include <sys/proc_info.h> | ||
| #include <sys/sysctl.h> | ||
|
|
||
| #include <algorithm> | ||
| #include <cerrno> | ||
| #include <cstddef> | ||
| #include <cstring> | ||
| #include <iterator> | ||
| #include <vector> | ||
|
|
||
| namespace nix { | ||
|
|
||
| void findDarwinRuntimeRoots(const StoreDirConfig & config, UncheckedRoots & unchecked) | ||
| { | ||
| auto storePathRegex = makeStorePathRegex(config); | ||
|
|
||
| std::vector<int> pids; | ||
| std::size_t pidBufSize = 1; | ||
|
|
||
| while (pidBufSize > pids.size() * sizeof(int)) { | ||
| // Reserve some extra size so we don't fail too much. | ||
| pids.resize((pidBufSize + pidBufSize / 8) / sizeof(int)); | ||
| auto size = proc_listpids(PROC_ALL_PIDS, 0, pids.data(), pids.size() * sizeof(int)); | ||
|
|
||
| if (size <= 0) | ||
| throw SysError("listing PIDs"); | ||
| pidBufSize = size; | ||
| } | ||
|
|
||
| pids.resize(pidBufSize / sizeof(int)); | ||
|
|
||
| for (auto pid : pids) { | ||
| checkInterrupt(); | ||
|
|
||
| // It doesn't make sense to ask about the kernel. | ||
| if (pid == 0) | ||
| continue; | ||
|
|
||
| try { | ||
| // Process cwd/root directory. | ||
| struct proc_vnodepathinfo vnodeInfo; | ||
| if (proc_pidinfo(pid, PROC_PIDVNODEPATHINFO, 0, &vnodeInfo, sizeof(vnodeInfo)) <= 0) | ||
| throw SysError("getting pid %1% working directory", pid); | ||
|
|
||
| unchecked[std::string(vnodeInfo.pvi_cdir.vip_path)].emplace(fmt("{libproc/%d/cwd}", pid)); | ||
| unchecked[std::string(vnodeInfo.pvi_rdir.vip_path)].emplace(fmt("{libproc/%d/rootdir}", pid)); | ||
|
|
||
| // File descriptors. | ||
| std::vector<struct proc_fdinfo> fds; | ||
| std::size_t fdBufSize = 1; | ||
| while (fdBufSize > fds.size() * sizeof(struct proc_fdinfo)) { | ||
| // Reserve some extra size so we don't fail too much. | ||
| fds.resize((fdBufSize + fdBufSize / 8) / sizeof(struct proc_fdinfo)); | ||
| errno = 0; | ||
| auto size = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, fds.data(), fds.size() * sizeof(struct proc_fdinfo)); | ||
|
|
||
| // The libproc wrapper converts a -1 syscall result to 0. A | ||
| // process with no file descriptors also returns 0, but leaves | ||
| // errno unchanged. | ||
| if (size <= 0) { | ||
| if (errno == 0) { | ||
| fdBufSize = 0; | ||
| break; | ||
| } | ||
| throw SysError("listing pid %1% file descriptors", pid); | ||
| } | ||
| fdBufSize = size; | ||
| } | ||
| fds.resize(fdBufSize / sizeof(struct proc_fdinfo)); | ||
|
|
||
| for (auto fd : fds) { | ||
| // By definition, only a vnode is on the filesystem. | ||
| if (fd.proc_fdtype != PROX_FDTYPE_VNODE) | ||
| continue; | ||
|
|
||
| struct vnode_fdinfowithpath fdInfo; | ||
| if (proc_pidfdinfo(pid, fd.proc_fd, PROC_PIDFDVNODEPATHINFO, &fdInfo, sizeof(fdInfo)) <= 0) { | ||
| // They probably just closed this fd, so continue looking | ||
| // at ranges and environment variables. | ||
| if (errno == EBADF) | ||
| continue; | ||
| throw SysError("getting pid %1% fd %2% path", pid, fd.proc_fd); | ||
| } | ||
|
|
||
| unchecked[std::string(fdInfo.pvip.vip_path)].emplace(fmt("{libproc/%d/fd/%d}", pid, fd.proc_fd)); | ||
| } | ||
|
|
||
| // Regions, including mmapped files, executables, and shared libraries. | ||
| uint64_t nextAddr = 0; | ||
| while (true) { | ||
| // PROC_PIDREGIONPATHINFO2 includes regions backed by a vnode | ||
| // and has been available since OS X 10.10, but is not exposed | ||
| // by the SDK headers. Its numeric value is 22. | ||
|
Comment on lines
+99
to
+100
Contributor
There was a problem hiding this comment. It's safe to drop this hack then since we are in the clear by having nixpkgs only support 14 and upwards? |
||
| struct proc_regionwithpathinfo regionInfo; | ||
| if (proc_pidinfo(pid, 22, nextAddr, ®ionInfo, sizeof(regionInfo)) <= 0) { | ||
| // The API signals the end of the region list as an error. | ||
| if (errno == ESRCH || errno == EINVAL) | ||
| break; | ||
| throw SysError("getting pid %1% region path", pid); | ||
| } | ||
|
|
||
| unchecked[std::string(regionInfo.prp_vip.vip_path)].emplace(fmt("{libproc/%d/region}", pid)); | ||
|
|
||
| nextAddr = regionInfo.prp_prinfo.pri_address + regionInfo.prp_prinfo.pri_size; | ||
| } | ||
|
|
||
| // Arguments and environment variables. Environment variables of | ||
| // entitled binaries cannot be read unless Nix has the | ||
| // com.apple.private.read-environment-variables entitlement or SIP | ||
| // is disabled. Arguments remain readable, but must be ignored: a | ||
| // path passed to `nix-store --delete` must not root itself. | ||
| int sysctlName[3] = {CTL_KERN, KERN_PROCARGS2, pid}; | ||
| size_t argsSize = 0; | ||
| if (sysctl(sysctlName, 3, nullptr, &argsSize, nullptr, 0) < 0) | ||
| throw SysError("reading pid %1% arguments", pid); | ||
|
|
||
| std::vector<char> args(argsSize); | ||
| if (sysctl(sysctlName, 3, args.data(), &argsSize, nullptr, 0) < 0) | ||
| throw SysError("reading pid %1% arguments", pid); | ||
|
|
||
| if (argsSize < args.size()) | ||
| args.resize(argsSize); | ||
|
|
||
| // The first four bytes contain argc, followed by the executable, | ||
| // argc arguments, then the environment. Skip the executable and | ||
| // arguments before searching for store paths. | ||
| if (args.size() < sizeof(int)) | ||
| continue; | ||
|
|
||
| int argc; | ||
| std::memcpy(&argc, args.data(), sizeof(argc)); | ||
| if (argc < 0) | ||
| continue; | ||
|
|
||
| auto argsIter = args.begin() + sizeof(argc); | ||
| // Skip the executable and any padding before argv. | ||
| argsIter = std::find(argsIter, args.end(), '\0'); | ||
| argsIter = std::find_if(argsIter, args.end(), [](char ch) { return ch != '\0'; }); | ||
|
|
||
| // Empty arguments are significant, so skip exactly one | ||
| // NUL-terminated entry for each argument. | ||
| for (int i = 0; argsIter != args.end() && i < argc; ++i) { | ||
| argsIter = std::find(argsIter, args.end(), '\0'); | ||
| if (argsIter != args.end()) | ||
| ++argsIter; | ||
| } | ||
|
|
||
| if (argsIter != args.end()) { | ||
| using RegexIterator = boost::regex_iterator<decltype(argsIter)>; | ||
| for (RegexIterator i(argsIter, args.end(), storePathRegex), end; i != end; ++i) | ||
| unchecked[i->str()].emplace(fmt("{libproc/%d/environ}", pid)); | ||
| } | ||
|
|
||
| // Per-thread working directories. | ||
| struct proc_taskallinfo taskAllInfo; | ||
| if (proc_pidinfo(pid, PROC_PIDTASKALLINFO, 0, &taskAllInfo, sizeof(taskAllInfo)) <= 0) | ||
| throw SysError("reading pid %1% tasks", pid); | ||
|
|
||
| // If the process doesn't have the per-thread cwd flag, the | ||
| // process-wide cwd above is sufficient. | ||
| if (taskAllInfo.pbsd.pbi_flags & PROC_FLAG_THCWD) { | ||
| std::vector<uint64_t> tids(taskAllInfo.ptinfo.pti_threadnum); | ||
| auto tidBufSize = | ||
| proc_pidinfo(pid, PROC_PIDLISTTHREADS, 0, tids.data(), tids.size() * sizeof(uint64_t)); | ||
| if (tidBufSize <= 0) | ||
| throw SysError("listing pid %1% threads", pid); | ||
|
|
||
| tids.resize(tidBufSize / sizeof(uint64_t)); | ||
| for (auto tid : tids) { | ||
| struct proc_threadwithpathinfo threadPathInfo; | ||
|
Contributor
There was a problem hiding this comment. Any particular reason why we need the C-style struct suffix? Seems like it could be safely dropped. I'd also maybe opt for explicitly qualifying with |
||
| if (proc_pidinfo(pid, PROC_PIDTHREADPATHINFO, tid, &threadPathInfo, sizeof(threadPathInfo)) <= 0) { | ||
| // The thread may have exited since it was listed. | ||
| if (errno == ESRCH) | ||
| continue; | ||
| throw SysError("reading pid %1% thread %2% cwd", pid, tid); | ||
| } | ||
|
|
||
| unchecked[std::string(threadPathInfo.pvip.vip_path)].emplace( | ||
| fmt("{libproc/%d/thread/%d/cwd}", pid, tid)); | ||
| } | ||
| } | ||
| } catch (SysError & e) { | ||
| // Processes can exit or become inaccessible at any point while | ||
| // they are inspected. Protected processes also reject some of | ||
| // these calls. | ||
| if (e.is(std::errc::no_such_file_or_directory) || e.is(std::errc::no_such_process) | ||
| || e.is(std::errc::invalid_argument) || e.is(std::errc::permission_denied) | ||
| || e.is(std::errc::operation_not_permitted) || e.is(std::errc::io_error)) | ||
|
Comment on lines
+183
to
+185
Contributor
There was a problem hiding this comment. Can we maybe extend the |
||
| continue; | ||
| throw; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace nix | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| include_dirs += [ include_directories('build') ] | ||
|
|
||
| sources += files( | ||
| 'build/darwin-derivation-builder.cc', | ||
| 'local-gc.cc', | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,4 @@ | ||
| #include "nix/store/gc-store.hh" | ||
|
|
||
| namespace nix { | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #pragma once | ||
|
|
||
| #include "nix/store/store-dir-config.hh" | ||
| #include "nix/util/types.hh" | ||
|
|
||
| #include <boost/regex.hpp> | ||
| #include <boost/unordered/unordered_flat_map.hpp> | ||
| #include <boost/unordered/unordered_flat_set.hpp> | ||
|
|
||
| namespace nix { | ||
|
|
||
| /** | ||
| * The key is a string because std::filesystem::path cannot be used here with | ||
| * macOS's libc++. | ||
| */ | ||
| using UncheckedRoots = boost::unordered_flat_map< | ||
| std::string, | ||
| boost::unordered_flat_set<std::string, StringViewHash, std::equal_to<>>, | ||
| StringViewHash, | ||
| std::equal_to<>>; | ||
|
|
||
| inline boost::regex makeStorePathRegex(const StoreDirConfig & config) | ||
| { | ||
| static auto specialRegex = boost::regex(R"([.^$\\*+?()\[\]{}|])"); | ||
| auto quotedStoreDir = boost::regex_replace(config.storeDir, specialRegex, R"(\\$&)"); | ||
| return boost::regex(quotedStoreDir + R"(/[0-9a-z]+[0-9a-zA-Z\+\-\._\?=]*)"); | ||
| } | ||
|
|
||
| void findDarwinRuntimeRoots(const StoreDirConfig & config, UncheckedRoots & unchecked); | ||
|
Contributor
There was a problem hiding this comment. I guess in the future we could also factor out Linux-specific code. Seems nice, though I'd probably rename this to something like |
||
|
|
||
| } // namespace nix | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious. I guess we could use this stuff in closeExtraFDs in process spawning machinery instead of the less efficient approach that it takes now.