bugfix: clear FD_CLOEXEC in child instead of parent before fork

src/mp/util.cpp

@@ -60,6 +60,18 @@ size_t MaxFd()
     }
 }
 
+//! Report an error and exit from the post-fork child of a multi-threaded
+//! process, where only async-signal-safe calls (like write and _exit) are
+//! allowed. Accepting only a reference to a char array (in practice a string
+//! literal) ensures no allocation is needed at the call site.
+template <std::size_t N>
+[[noreturn]] void ChildFail(const char (&msg)[N]) noexcept
+{
+    const ssize_t written = ::write(STDERR_FILENO, msg, N - 1);
+    (void)written;
+    _exit(126);
+}
+
 } // namespace
 
 std::string ThreadName(const char* exe_name)
@@ -130,11 +142,6 @@ std::tuple<ProcessId, SocketId> SpawnProcess(SpawnConnectInfoToArgsFn&& connect_
     const std::vector<std::string> args{connect_info_to_args(std::to_string(fds[0]))};
     const std::vector<char*> argv{MakeArgv(args)};
 
-    // Clear FD_CLOEXEC on fds[0] before forking so it survives exec in the child.
-    int fds0_flags;
-    KJ_SYSCALL(fds0_flags = fcntl(fds[0], F_GETFD));
-    KJ_SYSCALL(fcntl(fds[0], F_SETFD, fds0_flags & ~FD_CLOEXEC));
-
     ProcessId pid = fork();
     if (pid == -1) {
         throw std::system_error(errno, std::system_category(), "fork");
@@ -147,10 +154,7 @@ std::tuple<ProcessId, SocketId> SpawnProcess(SpawnConnectInfoToArgsFn&& connect_
             (void)close(fds[1]);
             throw std::system_error(errno, std::system_category(), "close");
         }
-        static constexpr char msg[] = "SpawnProcess(child): close(fds[1]) failed\n";
-        const ssize_t writeResult = ::write(STDERR_FILENO, msg, sizeof(msg) - 1);
-        (void)writeResult;
-        _exit(126);
+        ChildFail("SpawnProcess(child): close(fds[1]) failed\n");
     }
 
     if (!pid) {
@@ -163,6 +167,16 @@ std::tuple<ProcessId, SocketId> SpawnProcess(SpawnConnectInfoToArgsFn&& connect_
             }
         }
 
+        // Clear FD_CLOEXEC on socket 0 so it survives exec. Doing this here
+        // rather than in the parent before forking avoids a window where a
+        // concurrent fork in another thread would leak the descriptor into an
+        // unrelated child process (which would not clear it).
+        // fcntl is async-signal-safe.
+        const int fds0_flags = fcntl(fds[0], F_GETFD);
+        if (fds0_flags == -1 || fcntl(fds[0], F_SETFD, fds0_flags & ~FD_CLOEXEC) == -1) {
+            ChildFail("SpawnProcess(child): clearing FD_CLOEXEC failed\n");
+        }
+
         execvp(argv[0], argv.data());
         // NOTE: perror() is not async-signal-safe; calling it here in a
         // post-fork child may deadlock in multithreaded parents.