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-05-26 16:44:14-04:00
log389181f6be8810b5cd432e236a962229257a5b59
tree2130576f275126f3afc715c38161f90e10d0e920
parent591bbafee37126dab2e035be717bb928c5953155

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/process/Child.zig+12-14
......@@ -526,7 +526,18 @@ fn statusToTerm(status: u32) Term {
526526}
527527
528528fn spawnPosix(self: *ChildProcess) SpawnError!void {
529 const pipe_flags: posix.O = .{};
529 // The child process does need to access (one end of) these pipes. However,
530 // we must initially set CLOEXEC to avoid a race condition. If another thread
531 // is racing to spawn a different child process, we don't want it to inherit
532 // these FDs in any scenario; that would mean that, for instance, calls to
533 // `poll` from the parent would not report the child's stdout as closing when
534 // expected, since the other child may retain a reference to the write end of
535 // the pipe. So, we create the pipes with CLOEXEC initially. After fork, we
536 // need to do something in the new child to make sure we preserve the reference
537 // we want. We could use `fcntl` to remove CLOEXEC from the FD, but as it
538 // turns out, we `dup2` everything anyway, so there's no need!
539 const pipe_flags: posix.O = .{ .CLOEXEC = true };
540
530541 const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try posix.pipe2(pipe_flags) else undefined;
531542 errdefer if (self.stdin_behavior == StdIo.Pipe) {
532543 destroyPipe(stdin_pipe);
......@@ -614,19 +625,6 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void {
614625 setUpChildIo(self.stdout_behavior, stdout_pipe[1], posix.STDOUT_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err);
615626 setUpChildIo(self.stderr_behavior, stderr_pipe[1], posix.STDERR_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err);
616627
617 if (self.stdin_behavior == .Pipe) {
618 posix.close(stdin_pipe[0]);
619 posix.close(stdin_pipe[1]);
620 }
621 if (self.stdout_behavior == .Pipe) {
622 posix.close(stdout_pipe[0]);
623 posix.close(stdout_pipe[1]);
624 }
625 if (self.stderr_behavior == .Pipe) {
626 posix.close(stderr_pipe[0]);
627 posix.close(stderr_pipe[1]);
628 }
629
630628 if (self.cwd_dir) |cwd| {
631629 posix.fchdir(cwd.fd) catch |err| forkChildErrReport(err_pipe[1], err);
632630 } else if (self.cwd) |cwd| {