From b983c15336532f6a7b1b7089e1dfcfe5f6f90e66 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 7 Feb 2026 21:46:15 +0100 Subject: [PATCH] libutil: remove startProcess it's only used by runProgram2, and runProgram2 can do something much better. Change-Id: Idba563e1adbe3fd8ce07a0bb8ad4fc1b0cc057b9 --- lix/libstore/globals.cc | 28 +++++---- lix/libutil/processes.cc | 122 +++++++++++++++------------------------ lix/libutil/processes.hh | 17 ------ 3 files changed, 64 insertions(+), 103 deletions(-) diff --git a/lix/libstore/globals.cc b/lix/libstore/globals.cc index 165bc65c6..f6b73bb9f 100644 --- a/lix/libstore/globals.cc +++ b/lix/libstore/globals.cc @@ -261,17 +261,23 @@ StringSet Settings::getDefaultExtraPlatforms() if (!null) { throw Error("could not open /dev/null"); } - if (runProgram2(RunOptions{ - .program = "arch", - .args = {"-arch", "x86_64", "/usr/bin/true"}, - .redirections = - {{.dup = STDOUT_FILENO, .from = null.get()}, - {.dup = STDERR_FILENO, .from = null.get()}} - } - ).wait() - == 0) - { - extraPlatforms.insert("x86_64-darwin"); + try { + if (runProgram2( + RunOptions{ + .program = "/usr/bin/arch", + .searchPath = false, + .args = {"-arch", "x86_64", "/usr/bin/true"}, + .redirections = + {{.dup = STDOUT_FILENO, .from = null.get()}, + {.dup = STDERR_FILENO, .from = null.get()}} + } + ).wait() + == 0) + { + extraPlatforms.insert("x86_64-darwin"); + } + } catch (ExecError & e) { + debug("could not run /usr/bin/arch, not adding x86_64: %s", e.msg()); } } #endif diff --git a/lix/libutil/processes.cc b/lix/libutil/processes.cc index 277b82fb6..15b250de2 100644 --- a/lix/libutil/processes.cc +++ b/lix/libutil/processes.cc @@ -149,65 +149,6 @@ void killUser(uid_t uid) ////////////////////////////////////////////////////////////////////// - -static pid_t doFork(std::function fun) -{ - pid_t pid = fork(); - if (pid != 0) return pid; - fun(); - abort(); -} - -#if __linux__ -static int childEntry(void * arg) -{ - auto main = static_cast *>(arg); - (*main)(); - return 1; -} -#endif - - -Pid startProcess(std::function fun, const ProcessOptions & options) -{ - std::function wrapper = [&]() { - logger = makeSimpleLogger(); - try { - fun(); - } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) - try { - std::cerr << e.what() << "\n"; - } catch (...) { } - } catch (...) { } - _exit(1); - }; - - pid_t pid = -1; - - if (options.cloneFlags) { - #ifdef __linux__ - // Not supported, since then we don't know when to free the stack. - assert(!(options.cloneFlags & CLONE_VM)); - - size_t stackSize = 1ul * 1024 * 1024; - auto stack = static_cast(mmap(0, stackSize, - PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0)); - if (stack == MAP_FAILED) throw SysError("allocating stack"); - - Finally freeStack([&]() { munmap(stack, stackSize); }); - - pid = clone(childEntry, stack + stackSize, options.cloneFlags | SIGCHLD, &wrapper); - #else - throw Error("clone flags are only supported on Linux"); - #endif - } else - pid = doFork(wrapper); - - if (pid == -1) throw SysError("unable to fork"); - - return Pid{pid}; -} - kj::Promise> runProgram(Path program, bool searchPath, const Strings args, bool isInteractive) try { @@ -352,8 +293,6 @@ RunningProgram runProgram2(const RunOptions & options) Pipe out; if (options.captureStdout) out.create(); - ProcessOptions processOptions{}; - printMsg( lvlChatty, "running command%s%s: %s %s%s", @@ -364,9 +303,44 @@ RunningProgram runProgram2(const RunOptions & options) concatMapStringsSep(" ", options.args, shellEscape) ); + Pipe info; + info.create(); + /* Fork. */ - Pid pid{startProcess( - [&]() { + Pid pid{fork()}; + if (!pid) { + throw SysError("fork failed"); + } else if (pid.get() > 0) { + info.writeSide.close(); + auto status = readFile(info.readSide.get()); + if (!status.empty()) { + auto result = pid.kill(); + throw ExecError(result, "failed to run %s: %s", options.program, status); + } else { + return RunningProgram{ + options.program, + std::move(pid), + options.captureStdout ? std::move(out.readSide) : AutoCloseFD{} + }; + } + } else { + // nothing in the child may access global state like loggers, otherwise the + // child may deadlock if we forked while a lock was held by another thread. + + // we cannot use writeFull because it might throw, we want to exit instead. + auto writeStatus = [&](std::string_view data) { + while (!data.empty()) { + const auto sent = ::write(info.writeSide.get(), data.data(), data.size()); + if (sent < 0) { + _exit(255); + } + data.remove_prefix(sent); + } + }; + + try { + info.readSide.close(); + if (options.environment) { replaceEnv(*options.environment); } @@ -396,21 +370,19 @@ RunningProgram runProgram2(const RunOptions & options) sys::execvp(options.program, args_); // This allows you to refer to a program with a pathname relative // to the PATH variable. - } else + } else { sys::execv(options.program, args_); + } throw SysError("executing '%1%'", options.program); - }, - processOptions - )}; - - out.writeSide.close(); - - return RunningProgram{ - options.program, - std::move(pid), - options.captureStdout ? std::move(out.readSide) : AutoCloseFD{} - }; + } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) + writeStatus(e.what()); + _exit(254); + } catch (...) { + writeStatus("unknown exception"); + _exit(253); + } + } } RunningHelper runHelper(const char * name, RunOptions options) diff --git a/lix/libutil/processes.hh b/lix/libutil/processes.hh index a4808cd06..9d44f93d7 100644 --- a/lix/libutil/processes.hh +++ b/lix/libutil/processes.hh @@ -68,23 +68,6 @@ public: */ void killUser(uid_t uid); - -/** - * Fork a process that runs the given function, and return the child - * pid to the caller. - */ -struct ProcessOptions -{ - /** - * use clone() with the specified flags (Linux only) - */ - int cloneFlags = 0; -}; - -[[nodiscard]] -Pid startProcess(std::function fun, const ProcessOptions & options = ProcessOptions()); - - /** * Run a program and return its stdout in a string (i.e., like the * shell backtick operator).