authorgravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2024-05-26 09:40:49-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-06-06 10:43:14-07:00
log88146ea704f2b596b0bec0ab2aba786fcb636d8c
tree11ab8f2d6d9638f3d91070cd0d6842451f8b3250
parent6e469bc44d06037330c63d81f48904f37112f220

std.process.Child: prevent racing children from inheriting one another's pipes

The added comment explains the issue here relatively well. The new progress API made this bug obvious because it became visibly clear that certain Compile steps were seemingly "hanging" until other steps completed. As it turned out, these child processes had raced to spawn, and hence one had inherited the other's stdio pipes, meaning the `poll` call in `std.Build.Step.evalZigProcess` was not identifying the child stdout as closed until an unrelated process terminated.

1 files changed, 12 insertions(+), 14 deletions(-)

lib/std/child_process.zig+12-14
......@@ -529,7 +529,18 @@ pub const ChildProcess = struct {
529529 }
530530
531531 fn spawnPosix(self: *ChildProcess) SpawnError!void {
532 const pipe_flags: posix.O = .{};
532 // The child process does need to access (one end of) these pipes. However,
533 // we must initially set CLOEXEC to avoid a race condition. If another thread
534 // is racing to spawn a different child process, we don't want it to inherit
535 // these FDs in any scenario; that would mean that, for instance, calls to
536 // `poll` from the parent would not report the child's stdout as closing when
537 // expected, since the other child may retain a reference to the write end of
538 // the pipe. So, we create the pipes with CLOEXEC initially. After fork, we
539 // need to do something in the new child to make sure we preserve the reference
540 // we want. We could use `fcntl` to remove CLOEXEC from the FD, but as it
541 // turns out, we `dup2` everything anyway, so there's no need!
542 const pipe_flags: posix.O = .{ .CLOEXEC = true };
543
533544 const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try posix.pipe2(pipe_flags) else undefined;
534545 errdefer if (self.stdin_behavior == StdIo.Pipe) {
535546 destroyPipe(stdin_pipe);
......@@ -617,19 +628,6 @@ pub const ChildProcess = struct {
617628 setUpChildIo(self.stdout_behavior, stdout_pipe[1], posix.STDOUT_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err);
618629 setUpChildIo(self.stderr_behavior, stderr_pipe[1], posix.STDERR_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err);
619630
620 if (self.stdin_behavior == .Pipe) {
621 posix.close(stdin_pipe[0]);
622 posix.close(stdin_pipe[1]);
623 }
624 if (self.stdout_behavior == .Pipe) {
625 posix.close(stdout_pipe[0]);
626 posix.close(stdout_pipe[1]);
627 }
628 if (self.stderr_behavior == .Pipe) {
629 posix.close(stderr_pipe[0]);
630 posix.close(stderr_pipe[1]);
631 }
632
633631 if (self.cwd_dir) |cwd| {
634632 posix.fchdir(cwd.fd) catch |err| forkChildErrReport(err_pipe[1], err);
635633 } else if (self.cwd) |cwd| {