Skip to content

libstore: skip Spotlight processes when finding GC roots #13051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

libstore: skip Spotlight processes when finding GC roots #13051

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/libstore/gc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,13 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
if (getEnv("_NIX_TEST_NO_LSOF") != "1") {
try {
std::regex lsofRegex(R"(^n(/.*)$)");

// Despite installations of Nix explicitly setting the `nobrowse` mount
// option on the store, `mdworker` processes continue to run on newly
// created files within it, which can result in spurious GC/path deletion
// failures if the stars are aligned right.
auto lsofLines =
tokenizeString<std::vector<std::string>>(runProgram(LSOF, true, { "-n", "-w", "-F", "n" }), "\n");
tokenizeString<std::vector<std::string>>(runProgram(LSOF, true, { "-n", "-w", "-F", "n", "-u", "^89", "-g", "^89" }), "\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Echoing what @Qyriad said in https://gerrit.lix.systems/c/lix/+/3011/1/lix/libstore/platform/darwin.cc#53, the 89 should be pulled out into constant, with a name, and with (lots of!) docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't fully understand why excluding this user+processes from being GC roots makes GC more reliable.

Copy link
Member

@Mic92 Mic92 Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that uid/gid 89 is related to the mdworker? We should not check this process for live gcroots because that process shouldn't create a lock.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this should only be done on macOS, since the uid 89 may refer to something else on other systems.

Copy link
Member

@grahamc grahamc Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like lsof -u and -g supports taking names which I think would be better:
-u ^_spotlight -g ^_spotlight

grahamc@Grahams-MacBook-Pro detsys-ids-client % cat /etc/passwd | grep 89
_spotlight:*:89:89:Spotlight:/var/empty:/usr/bin/false
_biome:*:289:289:Biome:/var/db/biome:/usr/bin/false

grahamc@Grahams-MacBook-Pro detsys-ids-client % cat /etc/group  | grep 89
_spotlight:*:89:
_biome:*:289:_biome

ref:

       1) the `^' (negated) login name or user ID (UID), specified with the -u option;

       2) the `^' (negated) process ID (PID), specified with the -p option;

       3) the `^' (negated) process group ID (PGID), specified with the -g option;

       4) the `^' (negated) command, specified with the -c option;

       5) the (`^') negated TCP or UDP protocol state names, specified with the -s [p:s] option.

for (const auto & line : lsofLines) {
std::smatch match;
if (std::regex_match(line, match, lsofRegex))
Expand Down
Loading