From 0fc0c5a013c929649330e817a787195086e233bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Wed, 8 Jan 2025 18:15:55 +0100 Subject: [PATCH] Close non-stdio file handles when daemonizing Otherwise, when the compiler wrapper spawns the sccache server, the server may end up with unintended file descriptors, which can lead to unexpected problems. This is particularly problematic if any of those file descriptors that accidentally end up in the server process is a pipe, as the pipe will only be closed when all the processes with that file descriptor close it or exit. This was causing sccache to hang ninja, as ninja uses the EOF of a pipe given to the subprocess to detect when that subprocess has exited: https://github.com/ninja-build/ninja/issues/2444#issuecomment-2577999973 This patch adds a dependency on the [close_fds](https://crates.io/crates/close_fds) crate, which automatically chooses an appropriate mechanism to close a range of file descriptors. On Linux 5.9+ that mechanism will be libc::close_range(). Fixes https://github.com/mozilla/sccache/issues/2313 --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + src/util.rs | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8a79ec0ff..964504e22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -440,6 +440,16 @@ version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1" +[[package]] +name = "close_fds" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3bc416f33de9d59e79e57560f450d21ff8393adcf1cdfc3e6d8fb93d5f88a2ed" +dependencies = [ + "cfg-if 1.0.0", + "libc", +] + [[package]] name = "colorchoice" version = "1.0.0" @@ -2531,6 +2541,7 @@ dependencies = [ "cc", "chrono", "clap", + "close_fds", "daemonize", "directories", "encoding_rs", diff --git a/Cargo.toml b/Cargo.toml index d29cdbbb1..1226e2063 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -121,6 +121,7 @@ rouille = { version = "3.6", optional = true, default-features = false, features ] } syslog = { version = "6", optional = true } version-compare = { version = "0.1.1", optional = true } +close_fds = "0.3.2" [dev-dependencies] assert_cmd = "2.0.13" diff --git a/src/util.rs b/src/util.rs index 7174681d6..f0ca0f506 100644 --- a/src/util.rs +++ b/src/util.rs @@ -851,6 +851,10 @@ pub fn daemonize() -> Result<()> { Ok(ref val) if val == "1" => {} _ => { Daemonize::new().start().context("failed to daemonize")?; + // Be extra-zealous and clase all non-stdio file descriptors. + unsafe { + close_fds::close_open_fds(3, &[]); + } } }