fix: Disable connection pool to fix dist server feature (#1612)

Signed-off-by: Xuanwo <github@xuanwo.io>
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>

.github/actions/artifact_failure/action.yml

@@ -25,4 +25,6 @@ runs:
     - uses: actions/upload-artifact@v3
       with:
         name: ${{ inputs.name }}
-        path: target/failure-${{ inputs.name }}.tar.gz
+        path: |
+          target/failure-${{ inputs.name }}.tar.gz
+          /tmp/sccache_*.txt

.github/workflows/ci.yml

@@ -82,10 +82,10 @@ jobs:
         if: ${{ matrix.os == 'ubuntu-20.04' || matrix.os == 'ubuntu-22.04' }}
 
       - name: Build tests
-        run: cargo test --no-run --locked --all-targets --verbose ${{ matrix.extra_args }}
+        run: cargo test --no-run --locked --all-targets ${{ matrix.extra_args }}
 
       - name: Run tests
-        run: cargo test --locked --all-targets --verbose ${{ matrix.extra_args }}
+        run: cargo test --locked --all-targets ${{ matrix.extra_args }}
 
       - name: Upload failure
         if: failure()
@@ -150,7 +150,7 @@ jobs:
         if: ${{ matrix.target == 'aarch64-unknown-linux-musl' }}
 
       - name: Build
-        run: cargo build --locked --release --verbose --bin ${{ matrix.binary || 'sccache' }} --target ${{ matrix.target }} --features=openssl/vendored ${{ matrix.extra_args }}
+        run: cargo build --locked --release --bin ${{ matrix.binary || 'sccache' }} --target ${{ matrix.target }} --features=openssl/vendored ${{ matrix.extra_args }}
         env:
           CARGO_TARGET_AARCH64_UNKNOWN_LINUX_MUSL_LINKER: aarch64-linux-musl-gcc
           MACOSX_DEPLOYMENT_TARGET: ${{ matrix.macosx_deployment_target }}
@@ -211,7 +211,7 @@ jobs:
         run: cargo install grcov
 
       - name: Execute tests
-        run: cargo test --no-fail-fast --locked --all-targets --verbose ${{ matrix.extra_args }}
+        run: cargo test --no-fail-fast --locked --all-targets ${{ matrix.extra_args }}
         env:
           CARGO_INCREMENTAL: "0"
           RUSTC_WRAPPER: ""

src/bin/sccache-dist/token_check.rs

@@ -1,7 +1,7 @@
 use crate::jwt;
 use anyhow::{bail, Context, Result};
 use sccache::dist::http::{ClientAuthCheck, ClientVisibleMsg};
-use sccache::util::BASE64_URL_SAFE_ENGINE;
+use sccache::util::{new_reqwest_blocking_client, BASE64_URL_SAFE_ENGINE};
 use std::collections::HashMap;
 use std::result::Result as StdResult;
 use std::sync::Mutex;
@@ -97,7 +97,7 @@ impl MozillaCheck {
     pub fn new(required_groups: Vec<String>) -> Self {
         Self {
             auth_cache: Mutex::new(HashMap::new()),
-            client: reqwest::blocking::Client::new(),
+            client: new_reqwest_blocking_client(),
             required_groups,
         }
     }
@@ -266,7 +266,7 @@ impl ProxyTokenCheck {
         let maybe_auth_cache: Option<Mutex<(HashMap<String, Instant>, Duration)>> =
             cache_secs.map(|secs| Mutex::new((HashMap::new(), Duration::from_secs(secs))));
         Self {
-            client: reqwest::blocking::Client::new(),
+            client: new_reqwest_blocking_client(),
             maybe_auth_cache,
             url,
         }

src/compiler/compiler.rs

@@ -31,7 +31,6 @@ use crate::mock_command::{exit_status, CommandChild, CommandCreatorSync, RunComm
 use crate::util::{fmt_duration_as_secs, ref_env, run_input_output};
 use filetime::FileTime;
 use fs::File;
-#[cfg(feature = "dist-client")]
 use fs_err as fs;
 use std::borrow::Cow;
 use std::ffi::{OsStr, OsString};

src/dist/client_auth.rs

@@ -84,6 +84,7 @@ mod code_grant_pkce {
         html_response, json_response, query_pairs, MIN_TOKEN_VALIDITY, MIN_TOKEN_VALIDITY_WARNING,
         REDIRECT_WITH_AUTH_JSON,
     };
+    use crate::util::new_reqwest_blocking_client;
     use crate::util::BASE64_URL_SAFE_ENGINE;
     use futures::channel::oneshot;
     use hyper::{Body, Method, Request, Response, StatusCode};
@@ -242,7 +243,7 @@ mod code_grant_pkce {
             grant_type: GRANT_TYPE_PARAM_VALUE,
             redirect_uri,
         };
-        let client = reqwest::blocking::Client::new();
+        let client = new_reqwest_blocking_client();
         let res = client.post(token_url).json(&token_request).send()?;
         if !res.status().is_success() {
             bail!(

src/dist/http.rs

@@ -22,9 +22,6 @@ pub use self::server::{
 
 mod common {
     use http::header;
-    use http::header::CONNECTION;
-    use http::header::CONTENT_LENGTH;
-    use http::header::CONTENT_TYPE;
     #[cfg(feature = "dist-server")]
     use std::collections::HashMap;
     use std::fmt;
@@ -64,9 +61,12 @@ mod common {
             Ok(self.bytes(bytes))
         }
         fn bytes(self, bytes: Vec<u8>) -> Self {
-            self.header(CONTENT_TYPE, mime::APPLICATION_OCTET_STREAM.to_string())
-                .header(CONTENT_LENGTH, bytes.len())
-                .body(bytes)
+            self.header(
+                header::CONTENT_TYPE,
+                mime::APPLICATION_OCTET_STREAM.to_string(),
+            )
+            .header(header::CONTENT_LENGTH, bytes.len())
+            .body(bytes)
         }
         fn bearer_auth(self, token: String) -> Self {
             self.bearer_auth(token)
@@ -79,7 +79,7 @@ mod common {
     ) -> Result<T> {
         // Work around tiny_http issue #151 by disabling HTTP pipeline with
         // `Connection: close`.
-        let res = req.header(CONNECTION, "close").send().await?;
+        let res = req.header(header::CONNECTION, "close").send().await?;
 
         let status = res.status();
         let bytes = res.bytes().await?;
@@ -250,6 +250,7 @@ pub mod urls {
 #[cfg(feature = "dist-server")]
 mod server {
     use crate::jwt;
+    use crate::util::new_reqwest_blocking_client;
     use byteorder::{BigEndian, ReadBytesExt};
     use flate2::read::ZlibDecoder as ZlibReadDecoder;
     use rand::{rngs::OsRng, RngCore};
@@ -685,7 +686,7 @@ mod server {
                 check_server_auth,
             } = self;
             let requester = SchedulerRequester {
-                client: Mutex::new(reqwest::blocking::Client::new()),
+                client: Mutex::new(new_reqwest_blocking_client()),
             };
 
             macro_rules! check_server_auth_or_err {
@@ -939,14 +940,14 @@ mod server {
             let job_authorizer = JWTJobAuthorizer::new(jwt_key);
             let heartbeat_url = urls::scheduler_heartbeat_server(&scheduler_url);
             let requester = ServerRequester {
-                client: reqwest::blocking::Client::new(),
+                client: new_reqwest_blocking_client(),
                 scheduler_url,
                 scheduler_auth: scheduler_auth.clone(),
             };
 
             // TODO: detect if this panics
             thread::spawn(move || {
-                let client = reqwest::blocking::Client::new();
+                let client = new_reqwest_blocking_client();
                 loop {
                     trace!("Performing heartbeat");
                     match bincode_req(
@@ -1152,6 +1153,8 @@ mod client {
             let timeout = Duration::new(REQUEST_TIMEOUT_SECS, 0);
             let new_client_async = client_async_builder
                 .timeout(timeout)
+                // Disable keep-alive
+                .pool_max_idle_per_host(0)
                 .build()
                 .context("failed to create an async HTTP client")?;
             // Use the updated certificates

src/util.rs

@@ -507,6 +507,23 @@ pub fn daemonize() -> Result<()> {
     Ok(())
 }
 
+/// Disable connection pool to avoid broken connection between runtime
+///
+/// # TODO
+///
+/// We should refactor sccache current model to make sure that we only have
+/// one tokio runtime and keep reqwest alive inside it.
+///
+/// ---
+///
+/// More details could be found at https://github.com/mozilla/sccache/pull/1563
+pub fn new_reqwest_blocking_client() -> reqwest::blocking::Client {
+    reqwest::blocking::Client::builder()
+        .pool_max_idle_per_host(0)
+        .build()
+        .expect("http client must build with success")
+}
+
 #[cfg(test)]
 mod tests {
     use super::OsStrExt;

tests/dist.rs

@@ -25,7 +25,7 @@ mod harness;
 fn basic_compile(tmpdir: &Path, sccache_cfg_path: &Path, sccache_cached_cfg_path: &Path) {
     let envs: Vec<(_, &OsStr)> = vec![
         ("RUST_BACKTRACE", "1".as_ref()),
-        ("SCCACHE_LOG", "trace".as_ref()),
+        ("SCCACHE_LOG", "debug".as_ref()),
         ("SCCACHE_CONF", sccache_cfg_path.as_ref()),
         ("SCCACHE_CACHED_CONF", sccache_cached_cfg_path.as_ref()),
     ];

tests/harness/mod.rs

@@ -46,14 +46,15 @@ pub fn start_local_daemon(cfg_path: &Path, cached_cfg_path: &Path) {
     let _ = sccache_command()
         .arg("--start-server")
         // Uncomment following lines to debug locally.
-        // .env("SCCACHE_LOG", "debug")
-        // .env("SCCACHE_ERROR_LOG", "/tmp/sccache_log.txt")
+        .env("SCCACHE_LOG", "debug")
+        .env("SCCACHE_ERROR_LOG", "/tmp/sccache_local_daemon.txt")
         .env("SCCACHE_CONF", cfg_path)
         .env("SCCACHE_CACHED_CONF", cached_cfg_path)
         .status()
         .unwrap()
         .success();
 }
+
 pub fn stop_local_daemon() {
     trace!("sccache --stop-server");
     drop(
@@ -264,7 +265,7 @@ impl DistSystem {
                 "-e",
                 "SCCACHE_NO_DAEMON=1",
                 "-e",
-                "SCCACHE_LOG=sccache=trace",
+                "SCCACHE_LOG=debug",
                 "-e",
                 "RUST_BACKTRACE=1",
                 "--network",
@@ -332,7 +333,7 @@ impl DistSystem {
                 "--name",
                 &server_name,
                 "-e",
-                "SCCACHE_LOG=sccache=trace",
+                "SCCACHE_LOG=debug",
                 "-e",
                 "RUST_BACKTRACE=1",
                 "--network",