-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
base: master
Are you sure you want to change the base?
Conversation
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. IMO, the best way to fix this is to simply ignore any processes started by the Spotlight user, so that's exactly what we do here. Fixes: NixOS#6141 Change-Id: I6a6a636c70f3f443f07ba9280d5f1e04b2ed2985
0070b5f
to
c27b2a6
Compare
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"); |
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.
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.
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.
I still don't fully understand why excluding this user+processes from being GC roots makes GC more reliable.
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.
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.
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.
Also, this should only be done on macOS, since the uid 89 may refer to something else on other systems.
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.
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.
Modified from @winterqt 's patch:
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.IMO, the best way to fix this is to simply ignore any processes started by the Spotlight user, so that's exactly what we do here.
Fixes: #6141
Makes an assumption of the Spotlight uid/gid.