From fa4522c11c2f5de288f0dff37aecf29922b3d934 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 20 Jul 2023 19:47:30 +1000 Subject: [PATCH] Fix `Build::compile_objects` deadlock on parallel `jobserver::Client::acquire` may block if there's not enough token. To avoid blocking, this patch waits for child in a separate thread so that once the old compiler job is done, it may continue spawning new job instead of blocking forever. Signed-off-by: Jiahao XU --- src/lib.rs | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 412dc0701..519124114 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1258,7 +1258,7 @@ impl Build { #[cfg(feature = "parallel")] fn compile_objects(&self, objs: &[Object], print: &PrintThread) -> Result<(), Error> { - use std::sync::Once; + use std::sync::{mpsc::channel, Once}; // Limit our parallelism globally with a jobserver. Start off by // releasing our own token for this process so we can have a bit of an @@ -1266,7 +1266,8 @@ impl Build { // on Windows with the main implicit token, so we just have a bit extra // parallelism for a bit and don't reacquire later. let server = jobserver(); - let reacquire = server.release_raw().is_ok(); + // Reacquire our process's token on drop + let _reacquire = server.release_raw().ok().map(|_| JobserverToken(server)); // When compiling objects in parallel we do a few dirty tricks to speed // things up: @@ -1287,29 +1288,31 @@ impl Build { // acquire the appropriate tokens, Once all objects have been compiled // we wait on all the processes and propagate the results of compilation. - let children = objs - .iter() - .map(|obj| { - let (mut cmd, program) = self.create_compile_object_cmd(obj)?; - let token = server.acquire()?; + let (tx, rx) = channel::<(_, String, KillOnDrop, _)>(); - let child = spawn(&mut cmd, &program, print.pipe_writer_cloned()?.unwrap())?; + // Since jobserver::Client::acquire can block, waiting + // must be done in parallel so that acquire won't block forever. + let wait_thread = thread::Builder::new().spawn(move || { + for (cmd, program, mut child, _token) in rx { + wait_on_child(&cmd, &program, &mut child.0)?; + } - Ok((cmd, program, KillOnDrop(child), token)) - }) - .collect::, Error>>()?; + Ok(()) + })?; - for (cmd, program, mut child, _token) in children { - wait_on_child(&cmd, &program, &mut child.0)?; - } + for obj in objs { + let (mut cmd, program) = self.create_compile_object_cmd(obj)?; + let token = server.acquire()?; - // Reacquire our process's token before we proceed, which we released - // before entering the loop above. - if reacquire { - server.acquire_raw()?; + let child = spawn(&mut cmd, &program, print.pipe_writer_cloned()?.unwrap())?; + + if tx.send((cmd, program, KillOnDrop(child), token)).is_err() { + break; + } } + drop(tx); - return Ok(()); + return wait_thread.join().expect("wait_thread panics"); /// Returns a suitable `jobserver::Client` used to coordinate /// parallelism between build scripts. @@ -1365,6 +1368,13 @@ impl Build { child.kill().ok(); } } + + struct JobserverToken(&'static jobserver::Client); + impl Drop for JobserverToken { + fn drop(&mut self) { + let _ = self.0.acquire_raw(); + } + } } #[cfg(not(feature = "parallel"))]