From 6bc815456247292c5d770d9a54f0afbc99cf7326 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 17 Dec 2024 13:24:52 +0900 Subject: [PATCH] Always use sccache's own jobserver and close the pipes to the inherited jobserver. --- src/commands.rs | 4 +-- src/jobserver.rs | 70 +++++++++++++++++++++++++++++++++++++++++++---- src/server.rs | 2 +- src/test/tests.rs | 2 +- src/test/utils.rs | 2 +- src/util.rs | 5 ++++ 6 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 3f6857f62..0811b331b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -762,7 +762,7 @@ pub fn run_command(cmd: Command) -> Result { trace!("Command::PackageToolchain({})", executable.display()); let runtime = Runtime::new()?; - let jobserver = unsafe { Client::new() }; + let jobserver = Client::new(); let creator = ProcessCommandCreator::new(&jobserver); let args: Vec<_> = env::args_os().collect(); let env: Vec<_> = env::vars_os().collect(); @@ -788,7 +788,7 @@ pub fn run_command(cmd: Command) -> Result { env_vars, } => { trace!("Command::Compile {{ {:?}, {:?}, {:?} }}", exe, cmdline, cwd); - let jobserver = unsafe { Client::new() }; + let jobserver = Client::new(); let conn = connect_or_start_server(&get_addr(), startup_timeout)?; let mut runtime = Runtime::new()?; let res = do_compile( diff --git a/src/jobserver.rs b/src/jobserver.rs index c0df054db..c7ea6a931 100644 --- a/src/jobserver.rs +++ b/src/jobserver.rs @@ -8,6 +8,68 @@ use futures::StreamExt; use crate::errors::*; +// The execution model of sccache is that on the first run it spawns a server +// in the background and detaches it. +// When normally executing the rust compiler from either cargo or make, it +// will use cargo/make's jobserver and limit its resource usage accordingly. +// When executing the rust compiler through the sccache server, that jobserver +// is not available, and spawning as many rustc as there are CPUs can lead to +// a quadratic use of the CPU resources (each rustc spawning as many threads +// as there are CPUs). +// One way around this issue is to inherit the jobserver from cargo or make +// when the sccache server is spawned, but that means that in some cases, the +// cargo or make process can't terminate until the sccache server terminates +// after its idle timeout (which also never happens if SCCACHE_IDLE_TIMEOUT=0). +// Also, if the sccache server ends up shared between multiple runs of +// cargo/make, then which jobserver is used doesn't make sense anymore. +// Ideally, the sccache client would give a handle to the jobserver it has +// access to, so that the rust compiler would "just" use the jobserver it +// would have used if it had run without sccache, but that adds some extra +// complexity, and requires to use Unix domain sockets. +// What we do instead is to arbitrary use our own jobserver. +// Unfortunately, that doesn't absolve us from having to deal with the original +// jobserver, because make may give us file descriptors to its pipes, and the +// simple fact of keeping them open can block it. +// So if it does give us those file descriptors, close the preemptively. +// +// unsafe because it can use the wrong fds. +#[cfg(not(windows))] +pub unsafe fn discard_inherited_jobserver() { + if let Some(value) = ["CARGO_MAKEFLAGS", "MAKEFLAGS", "MFLAGS"] + .into_iter() + .find_map(|env| std::env::var(env).ok()) + { + if let Some(auth) = value.rsplit(' ').find_map(|arg| { + arg.strip_prefix("--jobserver-auth=") + .or_else(|| arg.strip_prefix("--jobserver-fds=")) + }) { + if !auth.starts_with("fifo:") { + let mut parts = auth.splitn(2, ','); + let read = parts.next().unwrap(); + let write = match parts.next() { + Some(w) => w, + None => return, + }; + let read = read.parse().unwrap(); + let write = write.parse().unwrap(); + if read < 0 || write < 0 { + return; + } + unsafe { + if libc::fcntl(read, libc::F_GETFD) == -1 { + return; + } + if libc::fcntl(write, libc::F_GETFD) == -1 { + return; + } + libc::close(read); + libc::close(write); + } + } + } + } +} + #[derive(Clone)] pub struct Client { helper: Option>, @@ -20,12 +82,8 @@ pub struct Acquired { } impl Client { - // unsafe because `from_env` is unsafe (can use the wrong fds) - pub unsafe fn new() -> Client { - match jobserver::Client::from_env() { - Some(c) => Client::_new(c, true), - None => Client::new_num(num_cpus::get()), - } + pub fn new() -> Client { + Client::new_num(num_cpus::get()) } pub fn new_num(num: usize) -> Client { diff --git a/src/server.rs b/src/server.rs index c61e52499..14207b48c 100644 --- a/src/server.rs +++ b/src/server.rs @@ -428,7 +428,7 @@ pub fn start_server(config: &Config, addr: &crate::net::SocketAddr) -> Result<() }); panic_hook(info) })); - let client = unsafe { Client::new() }; + let client = Client::new(); let runtime = tokio::runtime::Builder::new_multi_thread() .enable_all() .worker_threads(std::cmp::max(20, 2 * num_cpus::get())) diff --git a/src/test/tests.rs b/src/test/tests.rs index 3aa98a80f..fad140229 100644 --- a/src/test/tests.rs +++ b/src/test/tests.rs @@ -88,7 +88,7 @@ where CacheMode::ReadWrite, )); - let client = unsafe { Client::new() }; + let client = Client::new(); let srv = SccacheServer::new(0, runtime, client, dist_client, storage).unwrap(); let mut srv: SccacheServer<_, Arc>> = srv; let addr = srv.local_addr().unwrap(); diff --git a/src/test/utils.rs b/src/test/utils.rs index 19a182c8b..d8531be6c 100644 --- a/src/test/utils.rs +++ b/src/test/utils.rs @@ -83,7 +83,7 @@ macro_rules! assert_map_contains { } pub fn new_creator() -> Arc> { - let client = unsafe { Client::new() }; + let client = Client::new(); Arc::new(Mutex::new(MockCommandCreator::new(&client))) } diff --git a/src/util.rs b/src/util.rs index 001aedd2f..7174681d6 100644 --- a/src/util.rs +++ b/src/util.rs @@ -842,6 +842,7 @@ impl<'a> Hasher for HashToDigest<'a> { /// Pipe `cmd`'s stdio to `/dev/null`, unless a specific env var is set. #[cfg(not(windows))] pub fn daemonize() -> Result<()> { + use crate::jobserver::discard_inherited_jobserver; use daemonize::Daemonize; use std::env; use std::mem; @@ -853,6 +854,10 @@ pub fn daemonize() -> Result<()> { } } + unsafe { + discard_inherited_jobserver(); + } + static mut PREV_SIGSEGV: *mut libc::sigaction = 0 as *mut _; static mut PREV_SIGBUS: *mut libc::sigaction = 0 as *mut _; static mut PREV_SIGILL: *mut libc::sigaction = 0 as *mut _;