diff --git a/CHANGELOG.md b/CHANGELOG.md index e63126a..bccf298 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 2.6.2 + +## Fixed + * Non-empty connection pools were never dropped (#NNN) + # 2.6.1 ## Fixed diff --git a/Cargo.toml b/Cargo.toml index 72d4256..00dc9f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ureq" -version = "2.6.1" +version = "2.6.2" authors = ["Martin Algesten ", "Jacob Hoffman-Andrews "] description = "Simple, safe HTTP client" license = "MIT/Apache-2.0" diff --git a/src/agent.rs b/src/agent.rs index b78158e..4ed88ce 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -238,6 +238,10 @@ impl Agent { pub fn cookie_store(&self) -> CookieStoreGuard<'_> { self.state.cookie_tin.read_lock() } + + pub(crate) fn weak_state(&self) -> std::sync::Weak { + Arc::downgrade(&self.state) + } } const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 100; diff --git a/src/pool.rs b/src/pool.rs index 3eab4cf..a0668f8 100644 --- a/src/pool.rs +++ b/src/pool.rs @@ -3,6 +3,7 @@ use std::collections::{HashMap, VecDeque}; use std::io::{self, Read}; use std::sync::Mutex; +use crate::agent::AgentState; use crate::stream::Stream; use crate::{Agent, Proxy}; @@ -229,14 +230,17 @@ impl PoolKey { #[derive(Clone, Debug)] pub(crate) struct PoolReturner { - inner: Option<(Agent, PoolKey)>, + // We store a weak reference to an agent state here to avoid creating + // a reference loop, since AgentState contains a ConnectionPool, which + // contains Streams, which contain PoolReturners. + inner: Option<(std::sync::Weak, PoolKey)>, } impl PoolReturner { /// A PoolReturner that returns to the given Agent's Pool. - pub(crate) fn new(agent: Agent, pool_key: PoolKey) -> Self { + pub(crate) fn new(agent: &Agent, pool_key: PoolKey) -> Self { Self { - inner: Some((agent, pool_key)), + inner: Some((agent.weak_state(), pool_key)), } } @@ -246,8 +250,10 @@ impl PoolReturner { } pub(crate) fn return_to_pool(&self, stream: Stream) { - if let Some((agent, pool_key)) = &self.inner { - agent.state.pool.add(pool_key, stream); + if let Some((weak_state, pool_key)) = &self.inner { + if let Some(state) = weak_state.upgrade() { + state.pool.add(pool_key, stream); + } } } } @@ -433,7 +439,7 @@ mod tests { let agent = Agent::new(); let pool_key = PoolKey::new(&url, None); - let stream = NoopStream::stream(PoolReturner::new(agent.clone(), pool_key)); + let stream = NoopStream::stream(PoolReturner::new(&agent, pool_key)); let mut limited_read = LimitedRead::new(stream, std::num::NonZeroUsize::new(500).unwrap()); limited_read.read_exact(&mut out_buf).unwrap(); @@ -472,7 +478,7 @@ mod tests { let stream = Stream::new( ro, "1.1.1.1:4343".parse().unwrap(), - PoolReturner::new(agent.clone(), PoolKey::from_parts("http", "1.1.1.1", 8080)), + PoolReturner::new(&agent, PoolKey::from_parts("http", "1.1.1.1", 8080)), ); let chunked = crate::chunked::Decoder::new(stream); diff --git a/src/response.rs b/src/response.rs index 2bb667a..d5535d4 100644 --- a/src/response.rs +++ b/src/response.rs @@ -1161,10 +1161,7 @@ mod tests { let stream = Stream::new( test_stream, "1.1.1.1:4343".parse().unwrap(), - PoolReturner::new( - agent.clone(), - PoolKey::from_parts("https", "example.com", 443), - ), + PoolReturner::new(&agent, PoolKey::from_parts("https", "example.com", 443)), ); Response::do_from_stream( stream, diff --git a/src/stream.rs b/src/stream.rs index 828ce68..c444b4f 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -316,7 +316,7 @@ pub(crate) fn connect_http(unit: &Unit, hostname: &str) -> Result // let port = unit.url.port().unwrap_or(80); let pool_key = PoolKey::from_parts("http", hostname, port); - let pool_returner = PoolReturner::new(unit.agent.clone(), pool_key); + let pool_returner = PoolReturner::new(&unit.agent, pool_key); connect_host(unit, hostname, port).map(|(t, r)| Stream::new(t, r, pool_returner)) } @@ -328,7 +328,7 @@ pub(crate) fn connect_https(unit: &Unit, hostname: &str) -> Result