Use vfork() instead of fork() if available

Hopefully this reduces the chance of hitting ‘unable to fork: Cannot
allocate memory’ errors.  vfork() is used for everything except
starting builders.

5 files changed, 47 insertions, 22 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 6def0c1c54..3e67e55d4a 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -665,7 +665,7 @@ HookInstance::HookInstance()
     builderOut.create();
 
     /* Fork the hook. */
-    pid = fork();
+    pid = maybeVfork();
     switch (pid) {
 
     case -1:
@@ -2662,8 +2662,19 @@ void SubstitutionGoal::tryToRun()
     if (pathExists(destPath))
         deletePathWrapped(destPath);
 
+    worker.store.setSubstituterEnv();
+
+    /* Fill in the arguments. */
+    Strings args;
+    args.push_back(baseNameOf(sub));
+    args.push_back("--substitute");
+    args.push_back(storePath);
+    args.push_back(destPath);
+    const char * * argArr = strings2CharPtrs(args);
+
     /* Fork the substitute program. */
-    pid = fork();
+    pid = maybeVfork();
+
     switch (pid) {
 
     case -1:
@@ -2677,18 +2688,6 @@ void SubstitutionGoal::tryToRun()
             if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
                 throw SysError("cannot dup output pipe into stdout");
 
-            /* Pass configuration options (including those overriden
-               with --option) to the substituter. */
-            setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
-
-            /* Fill in the arguments. */
-            Strings args;
-            args.push_back(baseNameOf(sub));
-            args.push_back("--substitute");
-            args.push_back(storePath);
-            args.push_back(destPath);
-            const char * * argArr = strings2CharPtrs(args);
-
             execv(sub.c_str(), (char * *) argArr);
 
             throw SysError(format("executing `%1%'") % sub);
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index d6cdd10d6f..b4fc64d712 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -202,6 +202,7 @@ void checkStoreNotSymlink()
 
 
 LocalStore::LocalStore(bool reserveSpace)
+    : didSetSubstituterEnv(false)
 {
     schemaPath = settings.nixDBPath + "/schema";
 
@@ -943,6 +944,18 @@ Path LocalStore::queryPathFromHashPart(const string & hashPart)
 }
 
 
+void LocalStore::setSubstituterEnv()
+{
+    if (didSetSubstituterEnv) return;
+
+    /* Pass configuration options (including those overriden with
+       --option) to substituters. */
+    setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
+
+    didSetSubstituterEnv = true;
+}
+
+
 void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run)
 {
     if (run.pid != -1) return;
@@ -955,7 +968,9 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
     fromPipe.create();
     errorPipe.create();
 
-    run.pid = fork();
+    setSubstituterEnv();
+
+    run.pid = maybeVfork();
 
     switch (run.pid) {
 
@@ -964,10 +979,6 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
 
     case 0: /* child */
         try {
-            /* Pass configuration options (including those overriden
-               with --option) to the substituter. */
-            setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
-
             if (dup2(toPipe.readSide, STDIN_FILENO) == -1)
                 throw SysError("dupping stdin");
             if (dup2(fromPipe.writeSide, STDOUT_FILENO) == -1)
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 8d745cfb83..ebf3f6e2b4 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -208,6 +208,8 @@ public:
 
     void markContentsGood(const Path & path);
 
+    void setSubstituterEnv();
+
 private:
 
     Path schemaPath;
@@ -238,6 +240,8 @@ private:
     /* Cache for pathContentsGood(). */
     std::map<Path, bool> pathContentsGoodCache;
 
+    bool didSetSubstituterEnv;
+
     int getSchema();
 
     void openDB(bool create);
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 7e5d8bb805..bb59b09240 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -760,8 +760,8 @@ Pid::operator pid_t()
 
 void Pid::kill()
 {
-    if (pid == -1) return;
-    
+    if (pid == -1 || pid == 0) return;
+
     printMsg(lvlError, format("killing process %1%") % pid);
 
     /* Send the requested signal to the child.  If it has its own
@@ -883,7 +883,8 @@ string runProgram(Path program, bool searchPath, const Strings & args)
 
     /* Fork. */
     Pid pid;
-    pid = fork();
+    pid = maybeVfork();
+
     switch (pid) {
 
     case -1:
@@ -955,6 +956,13 @@ void setuidCleanup()
 }
 
 
+#if HAVE_VFORK
+pid_t (*maybeVfork)() = vfork;
+#else
+pid_t (*maybeVfork)() = fork;
+#endif
+
+
 //////////////////////////////////////////////////////////////////////
 
 
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 0e121ea5c6..90413b0efe 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -266,6 +266,9 @@ void closeOnExec(int fd);
    sanitize file handles 0, 1 and 2. */
 void setuidCleanup();
 
+/* Call vfork() if available, otherwise fork(). */
+extern pid_t (*maybeVfork)();
+
 
 /* User interruption. */