From 52c03bbd8690185a39552b37f4d9d8f0fd91dd64 Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Sun, 29 Jan 2023 00:46:27 +1100
Subject: [PATCH] Fix `Build::compile_objects` when used in parallel

Before this commit, each call to `Build::compile_objects` release a raw
token regardless of whether there is a token acquired by this process.

As such, if `Build::compile_objects` is used in multithreading context,
then it would call `Client::release_raw` for each thread and thus
increasing the parallelism far beyond the limit.

This commit fixed the bug by not acquiring the implicit global token
when initializing global `jobserver::Client` and only obtain token when
compiling objects.

It also replaced use of `Option<jobserver::Client>` with
`MaybeUninit<jobserver::Client>` and eliminate an `Option::unwrap()`
inside fn `jobserver`.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
 src/lib.rs | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index e295e9714..a47f6543e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1223,11 +1223,8 @@ impl Build {
 
         // 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.
+        // easier to write loop below.
         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:
@@ -1265,29 +1262,24 @@ 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()
             }
         }
 
@@ -1313,9 +1305,7 @@ impl Build {
 
             // 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);