Repository: seanmonstar/reqwest (language: rust)

Issue:
Proxy tests flaky because of uncoordinated environment access
I'm working on updating the package for reqwest on Fedora Linux to the latest version. I noticed new test failures in 0.12.9 compared to 0.12.8 in the `proxy` tests, and I see there were some proxy-related changes in 0.12.9, so I thought it worth reporting.

I am seeing failures in ~50% of test runs on `aarch64-unknown-linux-gnu`, and only on that architecture. I'm not sure why that might be the case, but our aarch64 builders are the ones that have the most multi-threading capability. The failed builds all had 12 CPU threads available, with tests running in parallel (`-j12`). We are running the equivalent of `cargo test --release`, if that matters.

I saw failures with various different Rust compiler versions (1.82, 1.81, 1.79, and 1.75) and on different OS versions (Fedora 42, 41, 40, 39, and RHEL 9 and CentOS Stream 10) so I don't think either of these play a role here.

All bad test runs I've seen were caused by one of these two failures:

```
failures:
---- system_http_proxy_basic_auth_parsed stdout ----
thread 'system_http_proxy_basic_auth_parsed' panicked at tests/proxy.rs:128:10:
called `Result::unwrap()` on an `Err` value: reqwest::Error { kind: Request, url: "http://hyper.rs/prox", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(IncompleteMessage)) }
```

```
failures:
---- test_using_system_proxy stdout ----
thread 'test_using_system_proxy' panicked at tests/proxy.rs:183:39:
called `Result::unwrap()` on an `Err` value: reqwest::Error { kind: Request, url: "http://not.a.real.sub.hyper.rs/prox", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(IncompleteMessage)) }
```



Discussion / hints:
Looks like those manipulate environment variables, so if they're run concurrently, they could clobber each other. Do you see the issues if the tests run with just 1 thread?

With v0.12.9, we removed an internal cache which made reqwest only read the environment variables once. The tests used to disable that progammatically, and also use some locks to prevent clobbering. I think those locks were lost in the fix, and probably need to be added back so concurrent tests can be run.
That would indeed match what I'm seeing - I will try running tests with `--test-threads=1` and report back.
Sorry for the delay - I've tried running the tests single-threaded and didn't see a single failure, so I assume that your guess about clobbered environment variables was correct :) For now I'll adapt our package to always run with `--test-threads=1`.

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index e2fd14b9f..7f5462825 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -95,9 +95,6 @@ __tls = ["dep:rustls-pemfile", "tokio/io-util"]
 __rustls = ["dep:hyper-rustls", "dep:tokio-rustls", "dep:rustls", "__tls", "dep:rustls-pemfile", "dep:rustls-pki-types"]
 __rustls-ring = ["hyper-rustls?/ring", "tokio-rustls?/ring", "rustls?/ring", "quinn?/ring"]
 
-# When enabled, disable using the cached SYS_PROXIES.
-__internal_proxy_sys_no_cache = []
-
 [dependencies]
 base64 = "0.22"
 http = "1"


Test patch:
diff --git a/tests/proxy.rs b/tests/proxy.rs
index 25d0f615c..cc32f04dd 100644
--- a/tests/proxy.rs
+++ b/tests/proxy.rs
@@ -5,6 +5,12 @@ use support::server;
 
 use std::env;
 
+use once_cell::sync::Lazy;
+use tokio::sync::Mutex;
+
+// serialize tests that read from / write to environment variables
+static HTTP_PROXY_ENV_MUTEX: Lazy<Mutex<()>> = Lazy::new(|| Mutex::new(()));
+
 #[tokio::test]
 async fn http_proxy() {
     let url = "http://hyper.rs/prox";
@@ -110,6 +116,9 @@ async fn system_http_proxy_basic_auth_parsed() {
         async { http::Response::default() }
     });
 
+    // avoid races with other tests that change "http_proxy"
+    let _env_lock = HTTP_PROXY_ENV_MUTEX.lock().await;
+
     // save system setting first.
     let system_proxy = env::var("http_proxy");
 
@@ -174,6 +183,9 @@ async fn test_using_system_proxy() {
         async { http::Response::default() }
     });
 
+    // avoid races with other tests that change "http_proxy"
+    let _env_lock = HTTP_PROXY_ENV_MUTEX.lock().await;
+
     // save system setting first.
     let system_proxy = env::var("http_proxy");
     // set-up http proxy.