From f363d958a7e245a684aef3802449d4f67959761d Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 5 Sep 2025 16:55:04 +0200
Subject: [PATCH] Fix hang in enterChroot() draining userNamespaceSync

Calling `drainFD()` will hang if another process has the write side
open, since then the child won't get an EOF. This can happen if we
have multiple threads doing a build, since in that case another thread
may fork a child process that inherits the write side of the first
thread.

We could set O_CLOEXEC on the write side (using pipe2()) but it won't
help here since we don't always do an exec() in the child, e.g. in the
case of builtin builders. (We need a "close-on-fork", not a
"close-on-exec".)
---
 .../unix/build/linux-derivation-builder.cc    | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc
index d474c001e..35730644b 100644
--- a/src/libstore/unix/build/linux-derivation-builder.cc
+++ b/src/libstore/unix/build/linux-derivation-builder.cc
@@ -362,9 +362,21 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
 
         userNamespaceSync.readSide = -1;
 
-        /* Close the write side to prevent runChild() from hanging
-           reading from this. */
-        Finally cleanup([&]() { userNamespaceSync.writeSide = -1; });
+        /* Make sure that we write *something* to the child in case of
+           an exception. Note that merely closing
+           `userNamespaceSync.writeSide` doesn't work in
+           multi-threaded Nix, since several child processes may have
+           inherited `writeSide` (and O_CLOEXEC doesn't help because
+           the children may not do an execve). */
+        bool userNamespaceSyncDone = false;
+        Finally cleanup([&]() {
+            try {
+                if (!userNamespaceSyncDone)
+                    writeFull(userNamespaceSync.writeSide.get(), "0\n");
+            } catch (...) {
+            }
+            userNamespaceSync.writeSide = -1;
+        });
 
         auto ss = tokenizeString<std::vector<std::string>>(readLine(sendPid.readSide.get()));
         assert(ss.size() == 1);
@@ -419,14 +431,15 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
             writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid));
 
         /* Signal the builder that we've updated its user namespace. */
-        writeFull(userNamespaceSync.writeSide.get(), "1");
+        writeFull(userNamespaceSync.writeSide.get(), "1\n");
+        userNamespaceSyncDone = true;
     }
 
     void enterChroot() override
     {
         userNamespaceSync.writeSide = -1;
 
-        if (drainFD(userNamespaceSync.readSide.get()) != "1")
+        if (readLine(userNamespaceSync.readSide.get()) != "1")
             throw Error("user namespace initialisation failed");
 
         userNamespaceSync.readSide = -1;