From c6718a9d95021476f0cca47b61479e4046090f4e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Wed, 6 Oct 2021 13:54:59 +0200
Subject: [PATCH] Don't reset the logger in a vfork

9c766a40cbbd3a350a9582d0fd8201e3361a63b2 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.
---
 src/libstore/build/local-derivation-goal.cc | 8 ++------
 src/libutil/util.cc                         | 8 ++------
 src/libutil/util.hh                         | 2 +-
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 4cb43bc11..3fc156108 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -755,7 +755,6 @@ void LocalDerivationGoal::startBuilder()
     result.startTime = time(0);
 
     /* Fork a child to build the package. */
-    ProcessOptions options;
 
 #if __linux__
     if (useChroot) {
@@ -798,8 +797,6 @@ void LocalDerivationGoal::startBuilder()
 
         userNamespaceSync.create();
 
-        options.allowVfork = false;
-
         Path maxUserNamespaces = "/proc/sys/user/max_user_namespaces";
         static bool userNamespacesEnabled =
             pathExists(maxUserNamespaces)
@@ -857,7 +854,7 @@ void LocalDerivationGoal::startBuilder()
             writeFull(builderOut.writeSide.get(),
                 fmt("%d %d\n", usingUserNamespace, child));
             _exit(0);
-        }, options);
+        });
 
         int res = helper.wait();
         if (res != 0 && settings.sandboxFallback) {
@@ -921,10 +918,9 @@ void LocalDerivationGoal::startBuilder()
 #endif
     {
     fallback:
-        options.allowVfork = !buildUser && !drv->isBuiltin();
         pid = startProcess([&]() {
             runChild();
-        }, options);
+        });
     }
 
     /* parent */
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 0b261ca60..563a72c12 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -939,9 +939,6 @@ void killUser(uid_t uid)
        users to which the current process can send signals.  So we
        fork a process, switch to uid, and send a mass kill. */
 
-    ProcessOptions options;
-    options.allowVfork = false;
-
     Pid pid = startProcess([&]() {
 
         if (setuid(uid) == -1)
@@ -964,7 +961,7 @@ void killUser(uid_t uid)
         }
 
         _exit(0);
-    }, options);
+    });
 
     int status = pid.wait();
     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
     // be shared (technically this is undefined, but in practice that's the
     // case), so we can't use it if we alter the environment
-    if (options.environment)
-        processOptions.allowVfork = false;
+    processOptions.allowVfork = !options.environment;
 
     /* Fork. */
     Pid pid = startProcess([&]() {
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 6d3e64949..29232453f 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -262,7 +262,7 @@ struct ProcessOptions
     string errorPrefix = "";
     bool dieWithParent = true;
     bool runExitHandlers = false;
-    bool allowVfork = true;
+    bool allowVfork = false;
 };
 
 pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());