fix(3p/nix): do not call vfork r/1457

The use of vfork() in Nix is entirely illegal. Quote:

If the process created by vfork() returns from the function in which vfork() was
called, or calls any other function before successfully calling _exit() or
one of the exec*() family of functions, the behavior is undefined.

-- Linux man-pages, release 5.05

Add a TODO to use the higher-performance variants of clone() on Linux when it
is available.

Change-Id: I42370e1568ad6e2d00d70d0b66c8aded8f1288bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1418
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Reviewed-by: Alyssa Ross <hi@alyssa.is>

2 files changed, 12 insertions, 17 deletions

diff --git a/third_party/nix/src/libutil/util.cc b/third_party/nix/src/libutil/util.cc
index bb6a17e03047..b97624e58dbb 100644
--- a/third_party/nix/src/libutil/util.cc
+++ b/third_party/nix/src/libutil/util.cc
@@ -859,7 +859,6 @@ void killUser(uid_t uid) {
      fork a process, switch to uid, and send a mass kill. */
 
   ProcessOptions options;
-  options.allowVfork = false;
 
   Pid pid = startProcess(
       [&]() {
@@ -897,16 +896,19 @@ void killUser(uid_t uid) {
 
 //////////////////////////////////////////////////////////////////////
 
-/* Wrapper around vfork to prevent the child process from clobbering
-   the caller's stack frame in the parent. */
-static pid_t doFork(bool allowVfork, const std::function<void()>& fun)
-    __attribute__((noinline));
-static pid_t doFork(bool allowVfork, const std::function<void()>& fun) {
+/*
+ * Please note that it is not legal for this function to call vfork().  If the
+ * process created by vfork() returns from the function in which vfork() was
+ * called, or calls any other function before successfully calling _exit() or
+ * one of the exec*() family of functions, the behavior is undefined.
+ */
+static pid_t doFork(const std::function<void()>& fun) __attribute__((noinline));
+static pid_t doFork(const std::function<void()>& fun) {
 #ifdef __linux__
-  pid_t pid = allowVfork ? vfork() : fork();
-#else
-  pid_t pid = fork();
+  // TODO(kanepyork): call clone() instead for faster forking
 #endif
+
+  pid_t pid = fork();
   if (pid != 0) {
     return pid;
   }
@@ -938,7 +940,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions& options) {
     }
   };
 
-  pid_t pid = doFork(options.allowVfork, wrapper);
+  pid_t pid = doFork(wrapper);
   if (pid == -1) {
     throw SysError("unable to fork");
   }
@@ -1012,12 +1014,6 @@ void runProgram2(const RunOptions& options) {
   }
 
   ProcessOptions processOptions;
-  // 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;
-  }
 
   /* Fork. */
   Pid pid = startProcess(
diff --git a/third_party/nix/src/libutil/util.hh b/third_party/nix/src/libutil/util.hh
index ede83164de2e..0fa15f745c86 100644
--- a/third_party/nix/src/libutil/util.hh
+++ b/third_party/nix/src/libutil/util.hh
@@ -232,7 +232,6 @@ struct ProcessOptions {
   std::string errorPrefix = "error: ";
   bool dieWithParent = true;
   bool runExitHandlers = false;
-  bool allowVfork = true;
 };
 
 pid_t startProcess(std::function<void()> fun,