diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-07-12 01:27:46 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-07-16 01:02:47 +0900 |
| commit | ba23ea4f22b5482d036706124a196fa03ef4b521 (patch) | |
| tree | f743427e11f8ad7e08a041900f1e6aa5b67a7cd0 /crates/shirabe/tests/util | |
| parent | e588920985c71eaa652a292db2dd1d75b73712e4 (diff) | |
| download | php-shirabe-ba23ea4f22b5482d036706124a196fa03ef4b521.tar.gz php-shirabe-ba23ea4f22b5482d036706124a196fa03ef4b521.tar.zst php-shirabe-ba23ea4f22b5482d036706124a196fa03ef4b521.zip | |
fix(proxy-manager): correct singleton lifecycle and simplify to a plain Mutex
reset() eagerly rebuilt the ProxyManager singleton immediately,
capturing env vars before a caller could set them for the next
request. PHP's reset() just nulls the static instance; getInstance()
lazily constructs on next use. Match that so proxy env vars set after
reset() are observed.
get_instance() also ensured the singleton was constructed under its
own lock, dropped that lock, and returned the bare Mutex; every caller
then took a second, independent lock. A reset() landing in that gap
would leave the caller observing None and panicking on
.as_ref().unwrap(), a state the old eager-reconstructing reset() could
not produce. Return the already-locked MutexGuard from get_instance()
instead, so construction and use happen under one lock, and update all
call sites accordingly.
Holding that guard across a loop body then deadlocked in
diagnose_command, since check_http_proxy transitively re-enters
get_instance() via HttpDownloader -> CurlDownloader, and
std::sync::Mutex is not reentrant. Re-acquire the lock fresh each
iteration with a short-lived guard instead.
Finally, Mutex::new is a const fn, so the OnceLock wrapper around it
was unnecessary indirection; a bare static Mutex<Option<ProxyManager>>
initializes to the same state without the get_or_init/get dance.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/tests/util')
| -rw-r--r-- | crates/shirabe/tests/util/http/proxy_manager_test.rs | 27 |
1 files changed, 9 insertions, 18 deletions
diff --git a/crates/shirabe/tests/util/http/proxy_manager_test.rs b/crates/shirabe/tests/util/http/proxy_manager_test.rs index 7beafe19..d37a1fd8 100644 --- a/crates/shirabe/tests/util/http/proxy_manager_test.rs +++ b/crates/shirabe/tests/util/http/proxy_manager_test.rs @@ -46,21 +46,18 @@ fn test_instantiation() { // PHP compares object identity (===); the value-based Rust singleton exposes a per-instance // generation id instead, which changes only when a new ProxyManager is constructed. let original_instance = { - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); guard.as_ref().unwrap().__generation() }; let same_instance = { - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); guard.as_ref().unwrap().__generation() }; assert_eq!(original_instance, same_instance); ProxyManager::reset(); let new_instance = { - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); guard.as_ref().unwrap().__generation() }; assert_ne!(same_instance, new_instance); @@ -74,8 +71,7 @@ fn test_get_proxy_for_request_throws_on_bad_proxy_url() { Platform::put_env("http_proxy", "localhost"); ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy_manager = guard.as_ref().unwrap(); assert!( @@ -126,8 +122,7 @@ fn test_lowercase_overrides_uppercase() { } ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy = guard.as_ref().unwrap().get_proxy_for_request(url).unwrap(); assert_eq!(expected_url, proxy.get_status(None).unwrap()); } @@ -161,8 +156,7 @@ fn test_cgi_proxy_is_only_used_when_no_http_proxy() { } ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy = guard .as_ref() .unwrap() @@ -180,8 +174,7 @@ fn test_no_http_proxy_does_not_use_https_proxy() { Platform::put_env("https_proxy", "https://proxy.com:443"); ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy = guard .as_ref() .unwrap() @@ -198,8 +191,7 @@ fn test_no_https_proxy_does_not_use_http_proxy() { Platform::put_env("http_proxy", "http://proxy.com:80"); ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy = guard .as_ref() .unwrap() @@ -280,8 +272,7 @@ fn test_get_proxy_for_request() { } ProxyManager::reset(); - let mutex = ProxyManager::get_instance(); - let guard = mutex.lock().unwrap(); + let guard = ProxyManager::get_instance(); let proxy = guard.as_ref().unwrap().get_proxy_for_request(url).unwrap(); assert_eq!(options.as_ref(), proxy.get_context_options()); |
