From 2b2936319e94d1e5b9ca8c4b0c84efe67ce4e7eb Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Tue, 14 Jul 2026 16:01:39 +0200 Subject: [PATCH 1/4] Fix async resolver worker deadlock --- src/resolver.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/resolver.rs b/src/resolver.rs index 2298a6b..4643c70 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -241,9 +241,13 @@ impl AsyncResolverWorker { "proxy resolver was dropped during async resolution".to_string(), )), }; - if let Some(sender) = lock(&worker_inflight).get(&request.key).cloned() { + // Remove while holding the lock, then notify after the guard + // is dropped. An `if let lock(...).get(...).cloned()` keeps + // the temporary guard alive through its body, so locking + // again there deadlocks this sole resolver thread. + let sender = lock(&worker_inflight).remove(&request.key); + if let Some(sender) = sender { let _ = sender.send(Some(result)); - lock(&worker_inflight).remove(&request.key); } } }) From aa3e89d0c9eb9ca4758e5bc817bbe2c7a4927df2 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Tue, 14 Jul 2026 16:17:11 +0200 Subject: [PATCH 2/4] Test async resolver worker completion --- src/resolver.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/resolver.rs b/src/resolver.rs index 4643c70..b0d6d65 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -864,6 +864,26 @@ mod tests { )); } + #[cfg(feature = "tokio")] + #[tokio::test(flavor = "current_thread")] + async fn async_resolution_worker_publishes_completed_result() { + let resolver = ProxyResolver::with_env( + ResolverOptions::default(), + env(&[("https_proxy", "http://proxy.example:3128")]), + ); + let target = url("https://example.com/"); + + let result = tokio::time::timeout( + Duration::from_secs(1), + resolver.resolve_proxy_async(&target), + ) + .await + .expect("async resolver worker did not publish its completed result") + .unwrap(); + + assert_eq!(result, vec![ProxyKind::Http("proxy.example:3128".into())]); + } + #[cfg(feature = "tokio")] #[tokio::test(flavor = "current_thread")] async fn async_resolution_coalesces_identical_concurrent_calls() { From e9afc5f13a50c18a2bdab4d1149553516eb9fc6f Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Tue, 14 Jul 2026 17:00:32 +0200 Subject: [PATCH 3/4] Test second async resolver request completion --- src/resolver.rs | 20 ----------- tests/async_resolution.rs | 75 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 tests/async_resolution.rs diff --git a/src/resolver.rs b/src/resolver.rs index b0d6d65..4643c70 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -864,26 +864,6 @@ mod tests { )); } - #[cfg(feature = "tokio")] - #[tokio::test(flavor = "current_thread")] - async fn async_resolution_worker_publishes_completed_result() { - let resolver = ProxyResolver::with_env( - ResolverOptions::default(), - env(&[("https_proxy", "http://proxy.example:3128")]), - ); - let target = url("https://example.com/"); - - let result = tokio::time::timeout( - Duration::from_secs(1), - resolver.resolve_proxy_async(&target), - ) - .await - .expect("async resolver worker did not publish its completed result") - .unwrap(); - - assert_eq!(result, vec![ProxyKind::Http("proxy.example:3128".into())]); - } - #[cfg(feature = "tokio")] #[tokio::test(flavor = "current_thread")] async fn async_resolution_coalesces_identical_concurrent_calls() { diff --git a/tests/async_resolution.rs b/tests/async_resolution.rs new file mode 100644 index 0000000..5b7c68b --- /dev/null +++ b/tests/async_resolution.rs @@ -0,0 +1,75 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +#![cfg(feature = "tokio")] + +use std::process::Command; +use std::thread; +use std::time::{Duration, Instant}; + +use os_proxy_resolver::{ProxyKind, ProxyResolver}; +use url::Url; + +const CHILD_ENV: &str = "OS_PROXY_RESOLVER_ASYNC_WORKER_CHILD"; + +/// Run the potentially wedging worker scenario in a subprocess: a deadlocked +/// `std::thread` keeps its process alive even after a Tokio timeout fires, so an +/// in-process timeout cannot fail cleanly. The parent can kill the child and +/// report a bounded test failure instead of hanging the whole suite. +#[test] +fn async_resolution_worker_processes_second_distinct_request() { + let mut child = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "async_resolution_distinct_request_child", + "--nocapture", + ]) + .env(CHILD_ENV, "1") + .env("https_proxy", "http://proxy.example:3128") + .env_remove("HTTPS_PROXY") + .env_remove("http_proxy") + .env_remove("HTTP_PROXY") + .env_remove("all_proxy") + .env_remove("ALL_PROXY") + .env_remove("no_proxy") + .env_remove("NO_PROXY") + .spawn() + .unwrap(); + + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if let Some(status) = child.try_wait().unwrap() { + assert!(status.success(), "async resolver child failed: {status}"); + return; + } + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + panic!("async resolver worker did not process its second distinct request"); + } + thread::sleep(Duration::from_millis(20)); + } +} + +#[test] +fn async_resolution_distinct_request_child() { + if std::env::var_os(CHILD_ENV).is_none() { + return; + } + + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let resolver = ProxyResolver::new(); + for host in ["first.example.com", "second.example.com"] { + let target = Url::parse(&format!("https://{host}/")).unwrap(); + assert_eq!( + resolver.resolve_proxy_async(&target).await.unwrap(), + vec![ProxyKind::Http("proxy.example:3128".into())] + ); + } + }); +} From 3616f030224fba3414b0da95239e8346fed20916 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Tue, 14 Jul 2026 17:15:46 +0200 Subject: [PATCH 4/4] Make async resolver regression portable --- tests/async_resolution.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/tests/async_resolution.rs b/tests/async_resolution.rs index 5b7c68b..53819f4 100644 --- a/tests/async_resolution.rs +++ b/tests/async_resolution.rs @@ -27,20 +27,21 @@ fn async_resolution_worker_processes_second_distinct_request() { "--nocapture", ]) .env(CHILD_ENV, "1") - .env("https_proxy", "http://proxy.example:3128") - .env_remove("HTTPS_PROXY") - .env_remove("http_proxy") - .env_remove("HTTP_PROXY") - .env_remove("all_proxy") - .env_remove("ALL_PROXY") - .env_remove("no_proxy") - .env_remove("NO_PROXY") .spawn() .unwrap(); let deadline = Instant::now() + Duration::from_secs(5); loop { if let Some(status) = child.try_wait().unwrap() { + // Cross-target CI launches this test through a Cargo runner (QEMU, + // Wine, ...), but a nested `Command` cannot recover that runner and + // may fail immediately when executing the foreign binary directly. + // Native jobs still exercise the regression; a deadlocked child is + // distinguished by remaining alive until the deadline below. + if status.code() == Some(2) { + eprintln!("skipping nested subprocess unsupported by this target runner"); + return; + } assert!(status.success(), "async resolver child failed: {status}"); return; } @@ -59,6 +60,24 @@ fn async_resolution_distinct_request_child() { return; } + // ProxyResolver snapshots its environment at construction. Clear every + // supported spelling before setting the canonical uppercase value last: + // Windows environment names are case-insensitive, so removing an uppercase + // alias after setting lowercase would remove the test value too. + for name in [ + "http_proxy", + "HTTP_PROXY", + "https_proxy", + "HTTPS_PROXY", + "all_proxy", + "ALL_PROXY", + "no_proxy", + "NO_PROXY", + ] { + std::env::remove_var(name); + } + std::env::set_var("HTTPS_PROXY", "http://proxy.example:3128"); + tokio::runtime::Builder::new_current_thread() .build() .unwrap()