From ea89df2b76811505239b508a570ac9c0ea591038 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 9 Nov 2012 18:00:33 +0100 Subject: [PATCH] Use vfork() instead of fork() if available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hopefully this reduces the chance of hitting ‘unable to fork: Cannot allocate memory’ errors. vfork() is used for everything except starting builders. --- configure.ac | 4 ++++ src/libstore/build.cc | 27 +++++++++++++-------------- src/libstore/local-store.cc | 21 ++++++++++++++++----- src/libstore/local-store.hh | 4 ++++ src/libutil/util.cc | 14 +++++++++++--- src/libutil/util.hh | 3 +++ 6 files changed, 51 insertions(+), 22 deletions(-) diff --git a/configure.ac b/configure.ac index 7333e03c04..0eb7d76325 100644 --- a/configure.ac +++ b/configure.ac @@ -115,6 +115,10 @@ AC_CHECK_HEADERS([sys/mount.h], [], [], ]) +# Check for vfork. +AC_FUNC_FORK() + + # Check for lutimes, optionally used for changing the mtime of # symlinks. AC_CHECK_FUNCS([lutimes]) 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 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. */