From 2e2086046168d0c57142cfb5f1ff458c43cb8b14 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 25 May 2026 09:20:30 -0700 Subject: [PATCH] Fix git hang caused by accidental inheritance of stdin FD (#57572) When restarting Zed, I hit a bug where all Git operations were hung. I realized that there was a hanging git process running `git cat-file --batch-check=%(objectname)`. The process was waiting on stdin. This was surprising, because [the code](https://github.com/zed-industries/zed/blob/e2bbdb19b6da2ee157ca1d36100acde2134a1663/crates/git/src/repository.rs#L1665-L1709) that spawns this process explicitly closes the pipe that is attached to the process's stdin after writing a list of ref names. Using Claude, I found that this could be caused by that pipe file descriptor being cloned due to file descriptor inheritance when another child process is `exec`'d while that stdin pipe is open. The fix is to enhance our Darwin process spawning layer to set the close-on-exec flag for the pipe file descriptors, so that they are not inherited by child processes spawned using code paths that don't set `POSIX_SPAWN_CLOEXEC_DEFAULT`. Release Notes: - Fixed a bug on macOS where Git operations could be blocked depending on the timing of spawning child processes. --- crates/util/src/command/darwin.rs | 80 +++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/crates/util/src/command/darwin.rs b/crates/util/src/command/darwin.rs index a3d7561f4e3..c9d5bc40aed 100644 --- a/crates/util/src/command/darwin.rs +++ b/crates/util/src/command/darwin.rs @@ -524,15 +524,42 @@ fn spawn_posix_spawn( fn create_pipe() -> io::Result<(libc::c_int, libc::c_int)> { let mut fds: [libc::c_int; 2] = [0; 2]; - let result = unsafe { libc::pipe(fds.as_mut_ptr()) }; - if result == -1 { - return Err(io::Error::last_os_error()); + unsafe { + let result = libc::pipe(fds.as_mut_ptr()); + if result == -1 { + let error = io::Error::last_os_error(); + return Err(error); + } + + // Set close-on-exec on both ends of the pipe. + // + // Without this, unrelated spawns elsewhere in the process (e.g. + // `smol::process` or `async_process`, which on Apple platforms use + // `posix_spawn` *without* `POSIX_SPAWN_CLOEXEC_DEFAULT`) would inherit + // these file descriptors and keep the pipes open even after we drop our + // side. + for &fd in &fds { + let result = libc::ioctl(fd, libc::FIOCLEX); + if result == -1 { + let error = io::Error::last_os_error(); + libc::close(fds[0]); + libc::close(fds[1]); + return Err(error); + } + } + + Ok((fds[0], fds[1])) } - Ok((fds[0], fds[1])) } fn open_dev_null(flags: libc::c_int) -> io::Result { - let fd = unsafe { libc::open(c"/dev/null".as_ptr() as *const libc::c_char, flags) }; + // Set close-on-exec for this pipe, for the same reason as in `create_pipe`. + let fd = unsafe { + libc::open( + c"/dev/null".as_ptr() as *const libc::c_char, + flags | libc::O_CLOEXEC, + ) + }; if fd == -1 { return Err(io::Error::last_os_error()); } @@ -561,6 +588,49 @@ mod tests { use super::*; use futures_lite::AsyncWriteExt; + // Verifies that pipes returned by `create_pipe` aren't visible to unrelated + // child processes spawned via `std::process::Command`. On macOS, `std` + // uses `posix_spawn` without `POSIX_SPAWN_CLOEXEC_DEFAULT`, so any + // non-CLOEXEC fd in the parent leaks into the child. Without + // `FD_CLOEXEC` on our pipe fds, an unrelated spawn (a terminal, the crash + // handler, etc.) running concurrently with a piped git child would hold + // git's stdin write end open and deadlock the git child on `read()`. + #[test] + fn test_create_pipe_not_inherited_by_unrelated_spawn() { + let (read_fd, write_fd) = create_pipe().expect("create_pipe failed"); + + // Probe with the exact fds returned by `create_pipe` (no dup), since + // duping with `F_DUPFD` would lose CLOEXEC and `F_DUPFD_CLOEXEC` would + // unconditionally set it, either of which would defeat the test. + #[allow(clippy::disallowed_methods)] + let output = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(format!( + "for fd in {read_fd} {write_fd}; do \ + if [ -e /dev/fd/$fd ]; then \ + echo $fd WAS INHERITED; \ + else \ + echo $fd WAS NOT INHERITED; \ + fi; \ + done; \ + echo DONE" + )) + .output() + .expect("failed to spawn sh"); + + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + + unsafe { + libc::close(read_fd); + libc::close(write_fd); + } + + assert_eq!( + stdout, + format!("{read_fd} WAS NOT INHERITED\n{write_fd} WAS NOT INHERITED\nDONE\n") + ); + } + #[test] fn test_spawn_echo() { smol::block_on(async {