Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Build::compile_objects on error and when used in multithreading #779

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1221,13 +1221,8 @@ impl Build {
fn compile_objects(&self, objs: &[Object]) -> Result<(), Error> {
use std::sync::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
// easier to write loop below. If this fails, though, then we're likely
// on Windows with the main implicit token, so we just have a bit extra
// parallelism for a bit and don't reacquire later.
// Limit our parallelism globally with a jobserver.
let server = jobserver();
let reacquire = server.release_raw().is_ok();

// When compiling objects in parallel we do a few dirty tricks to speed
// things up:
Expand Down Expand Up @@ -1265,36 +1260,43 @@ impl Build {
wait_on_child(&cmd, &program, &mut child.0)?;
}

// Reacquire our process's token before we proceed, which we released
// before entering the loop above.
if reacquire {
server.acquire_raw()?;
}

return Ok(());

/// Returns a suitable `jobserver::Client` used to coordinate
/// parallelism between build scripts.
fn jobserver() -> &'static jobserver::Client {
use std::mem::MaybeUninit;

static INIT: Once = Once::new();
static mut JOBSERVER: Option<jobserver::Client> = None;
static mut JOBSERVER: MaybeUninit<jobserver::Client> = MaybeUninit::uninit();

fn _assert_sync<T: Sync>() {}
_assert_sync::<jobserver::Client>();

unsafe {
INIT.call_once(|| {
let server = default_jobserver();
JOBSERVER = Some(server);
JOBSERVER.write(default_jobserver());
});
JOBSERVER.as_ref().unwrap()
JOBSERVER.assume_init_ref()
}
}

unsafe fn default_jobserver() -> jobserver::Client {
// Try to use the environmental jobserver which Cargo typically
// initializes for us...
if let Some(client) = jobserver::Client::from_env() {
match client.available() {
Ok(0) | Err(_) => {
// There is no available tokens.
// This is probably because `cc::Build::compile_objects` is called in
// `build.rs` where the `cargo` holds the token.
//
// In that case, we will release one raw token just to prevent deadlock,
// but this will increase the allowed global concurrency by one.
let _ = client.release_raw();
}
Comment on lines +1288 to +1297
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshtriplett I implemented what you requested (only acquire and release token when actually compiling) but with a catch.

cargo would acquire one token before running the build.rs, so the available token could be 0 when we obtained the jobserver via jobserver::Client::from_env and causing deadlock.

To prevent this, I added this piece of code which release one token into the jobserver, but it cannot be re-acquired unless I added back the global state for the obtained token.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the CI is still in deadlock.

_ => (),
}
return client;
}

Expand All @@ -1307,15 +1309,15 @@ impl Build {
let mut parallelism = 4;
if let Ok(amt) = env::var("NUM_JOBS") {
if let Ok(amt) = amt.parse() {
parallelism = amt;
if amt > 0 {
parallelism = amt;
}
}
}

// If we create our own jobserver then be sure to reserve one token
// for ourselves.
let client = jobserver::Client::new(parallelism).expect("failed to create jobserver");
client.acquire_raw().expect("failed to acquire initial");
return client;
jobserver::Client::new(parallelism).expect("failed to create jobserver")
}

struct KillOnDrop(Child);
Expand Down
Loading