libstore: move build hook launching to libexec helper

like diff hooks before the build hooks also use runProgram features that
aren't present in posix_spawn. just like diff hooks we do not expect the
build hook processes to be so fast that launch overhead matters somehow.

Change-Id: If3c33dbd7d2ac20c95886e06d24feda312946c78

lix/libexec/meson.build

@@ -14,6 +14,13 @@ kill_user = executable(
   install_dir : libexecdir / 'lix',
 )
 
+run_build_hook = executable(
+  'run-build-hook',
+  files('run-build-hook.cc'),
+  install : true,
+  install_dir : libexecdir / 'lix',
+)
+
 run_diff_hook = executable(
   'run-diff-hook',
   files('run-diff-hook.cc'),

lix/libexec/run-build-hook.cc

@@ -0,0 +1,17 @@
+#include "common.hh"
+#include <unistd.h>
+
+LIBEXEC_HELPER(2)
+
+int helperMain(const char * name, std::span<char *> args) noexcept
+{
+    DIE_UNLESS_SYS("chdir", chdir("/"));
+    DIE_UNLESS_SYS("setsid", setsid());
+
+    static_assert(STDIN_FILENO == 0);
+    DIE_UNLESS_SYS("close(stdin)", close(STDIN_FILENO));
+    DIE_UNLESS_SYS("stdin = open(/dev/null)", open("/dev/null", O_RDWR));
+
+    execv(args[0], args.subspan(1).data());
+    die("exec failed");
+}

lix/libstore/build/hook-instance.cc

@@ -15,6 +15,7 @@
 #include "lix/libutil/logging-rpc.hh" // IWYU pragma: keep
 #include "lix/libutil/types-rpc.hh" // IWYU pragma: keep
 #include <fcntl.h>
+#include <kj/common.h>
 #include <kj/memory.h>
 #include <memory>
 #include <string_view>
@@ -74,32 +75,27 @@ try {
         throw Error("'build-hook' setting is empty");
 
     auto buildHook = canonPath(buildHookArgs.front());
-    buildHookArgs.pop_front();
+    buildHookArgs.emplace(std::next(buildHookArgs.begin()), baseNameOf(buildHook));
+    buildHookArgs.push_back(std::to_string(verbosity));
 
     /* Create the communication pipes. */
     auto [selfRPC, hookRPC] = SocketPair::stream();
 
-    AutoCloseFD devNull(open("/dev/null", O_RDWR | O_CLOEXEC));
-    if (!devNull) {
-        throw SysError("cannot open /dev/null");
-    }
-    RunOptions options{
-        .program = buildHook,
-        .searchPath = false,
-        .argv0 = std::string(baseNameOf(buildHook)),
-        .args = buildHookArgs,
-        .chdir = "/",
-        .createSession = true,
-        .redirections = {
-            {.dup = STDIN_FILENO, .from = devNull.get()},
-            {.dup = STDOUT_FILENO, .from = hookRPC.get()},
-        },
-    };
-
-    options.args.push_back(std::to_string(verbosity));
-
     /* Fork the hook. */
-    auto [pid, _stdout] = runProgram2(options).release();
+    auto pid = runHelper(
+        "run-build-hook",
+        {
+            .args = buildHookArgs,
+            .redirections = {{.dup = STDOUT_FILENO, .from = hookRPC.get()}},
+        }
+    );
+    KJ_DEFER({
+        // kill the hook if the promise is cancelled. the hook helper creates
+        // a session, so we'll kill the entire process group just to be safe.
+        if (pid) {
+            pid.killProcessGroup();
+        }
+    });
 
     std::map<std::string, Config::SettingInfo> settings;
     globalConfig.getSettings(settings, true);
@@ -116,7 +112,7 @@ try {
     }
 
     co_return std::make_unique<HookInstance>(
-        kj::heap(std::move(rpc)).attach(std::move(conn), std::move(client)), ProcessGroup(std::move(pid))
+        kj::heap(std::move(rpc)).attach(std::move(conn), std::move(client)), std::move(pid)
     );
 } catch (...) {
     co_return result::current_exception();

lix/libstore/build/hook-instance.hh

@@ -34,42 +34,41 @@ struct HookInstance
 
     static kj::Promise<Result<std::unique_ptr<HookInstance>>> create(const Activity & act);
 
-    HookInstance(kj::Own<rpc::build_remote::HookInstance::Client> rpc, ProcessGroup pg)
+    HookInstance(kj::Own<rpc::build_remote::HookInstance::Client> rpc, RunningHelper hook)
         : rpc(std::move(rpc))
-        , pgOrStatus(std::move(pg))
+        , hookOrStatus(std::move(hook))
     {
     }
     ~HookInstance();
 
     int wait()
     {
-        return childStatusOr<&ProcessGroup::wait>();
+        return childStatusOr([](auto & p) { return p.wait(); });
     }
 
     int kill()
     {
-        return childStatusOr<&ProcessGroup::kill>();
+        return childStatusOr([](auto & p) { return p.killProcessGroup(); });
     }
 
 private:
     /**
-     * The process group of the hook if it's running, or its exit status if not.
+     * The process of the hook if it's running, or its exit status if not.
      */
-    std::variant<ProcessGroup, int> pgOrStatus;
+    std::variant<RunningHelper, int> hookOrStatus;
 
-    template<int (ProcessGroup::*fn)()>
-    int childStatusOr()
+    int childStatusOr(auto ifRunning)
     {
         return std::visit(
             overloaded{
-                [&](ProcessGroup & pg) {
-                    int status = (pg.*fn)();
-                    pgOrStatus = status;
+                [&](RunningHelper & hook) {
+                    int status = ifRunning(hook);
+                    hookOrStatus = status;
                     return status;
                 },
                 [](int status) { return status; },
             },
-            pgOrStatus
+            hookOrStatus
         );
     }
 };

lix/libutil/processes.cc

@@ -288,6 +288,11 @@ int RunningProgram::wait()
     return pid.wait();
 }
 
+int RunningHelper::killProcessGroup()
+{
+    return ProcessGroup{std::move(pid)}.kill();
+}
+
 void RunningProgram::waitAndCheck()
 {
     if (std::uncaught_exceptions() == 0) {

lix/libutil/processes.hh

@@ -123,7 +123,7 @@ struct [[nodiscard("you must call RunningProgram::wait()")]] RunningProgram
 {
     friend RunningProgram runProgram2(const RunOptions & options);
 
-private:
+protected:
     Path program;
     Pid pid;
     std::unique_ptr<AsyncFdIoStream> childStdout;
@@ -187,6 +187,12 @@ public:
     using RunningProgram::kill;
     using RunningProgram::wait;
 
+    /**
+     * Kill the entire process group the helper runs in. This is usually **unsafe**
+     * unless the helper process has made itself a process group or session leader!
+     */
+    int killProcessGroup();
+
     void waitAndCheck();
 };