lix-project/lix
20
55
Fork 31
Code Issues 344 Code Review (Gerrit) Projects 3 Releases Wiki Activity

Don't reset the logger in a vfork

Browse source 
9c766a40cb broke logging from the
daemon, because commonChildInit is called when starting the build hook
in a vfork, so it ends up resetting the parent's logger. So don't
vfork.

It might be best to get rid of vfork altogether, but that may cause
problems, e.g. when we call an external program like git from the
evaluator.
This commit is contained in:
Eelco Dolstra 2021-10-06 13:54:59 +02:00
parent ce9823d9b7
commit c6718a9d95
3 changed files with 5 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/libstore/build/local-derivation-goal.cc
View file

@ -755,7 +755,6 @@ void LocalDerivationGoal::startBuilder()
result.startTime = time(0); result.startTime = time(0);
/* Fork a child to build the package. */ /* Fork a child to build the package. */
ProcessOptions options;
#if __linux__ #if __linux__
if (useChroot) { if (useChroot) {
@ -798,8 +797,6 @@ void LocalDerivationGoal::startBuilder()
userNamespaceSync.create(); userNamespaceSync.create();
options.allowVfork = false;
Path maxUserNamespaces = "/proc/sys/user/max_user_namespaces"; Path maxUserNamespaces = "/proc/sys/user/max_user_namespaces";
static bool userNamespacesEnabled = static bool userNamespacesEnabled =
pathExists(maxUserNamespaces) pathExists(maxUserNamespaces)
@ -857,7 +854,7 @@ void LocalDerivationGoal::startBuilder()
writeFull(builderOut.writeSide.get(), writeFull(builderOut.writeSide.get(),
fmt("%d %d\n", usingUserNamespace, child)); fmt("%d %d\n", usingUserNamespace, child));
_exit(0); _exit(0);
}, options); });
int res = helper.wait(); int res = helper.wait();
if (res != 0 && settings.sandboxFallback) { if (res != 0 && settings.sandboxFallback) {
@ -921,10 +918,9 @@ void LocalDerivationGoal::startBuilder()
#endif #endif
{ {
fallback: fallback:
options.allowVfork = !buildUser && !drv->isBuiltin();
pid = startProcess([&]() { pid = startProcess([&]() {
runChild(); runChild();
}, options); });
} }
/* parent */ /* parent */

8
src/libutil/util.cc
View file

@ -939,9 +939,6 @@ void killUser(uid_t uid)
users to which the current process can send signals. So we users to which the current process can send signals. So we
fork a process, switch to uid, and send a mass kill. */ fork a process, switch to uid, and send a mass kill. */
ProcessOptions options;
options.allowVfork = false;
Pid pid = startProcess([&]() { Pid pid = startProcess([&]() {
if (setuid(uid) == -1) if (setuid(uid) == -1)
@ -964,7 +961,7 @@ void killUser(uid_t uid)
} }
_exit(0); _exit(0);
}, options); });
int status = pid.wait(); int status = pid.wait();
if (status != 0) if (status != 0)
@ -1085,8 +1082,7 @@ void runProgram2(const RunOptions & options)
// vfork implies that the environment of the main process and the fork will // vfork implies that the environment of the main process and the fork will
// be shared (technically this is undefined, but in practice that's the // be shared (technically this is undefined, but in practice that's the
// case), so we can't use it if we alter the environment // case), so we can't use it if we alter the environment
if (options.environment) processOptions.allowVfork = !options.environment;
processOptions.allowVfork = false;
/* Fork. */ /* Fork. */
Pid pid = startProcess([&]() { Pid pid = startProcess([&]() {

2
src/libutil/util.hh
View file

@ -262,7 +262,7 @@ struct ProcessOptions
string errorPrefix = ""; string errorPrefix = "";
bool dieWithParent = true; bool dieWithParent = true;
bool runExitHandlers = false; bool runExitHandlers = false;
bool allowVfork = true; bool allowVfork = false;
}; };
pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions()); pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());