Model downloads: Time out and cancel the name lookup (#1532)

The host name lookup blocked with no deadline before the cancel check, and each address then got the full connect timeout. The lookup now runs on a helper thread under the connect deadline, which all addresses share.
This commit is contained in:
Michael Anderson
2026-10-14 13:56:34 -05:00
parent 3b4279fc24
commit e65b97e239
2 changed files with 109 additions and 20 deletions
+104 -20
View File
@@ -10,7 +10,7 @@
//! the downloading thread. No proxies (see docs/ai-masks.md).
use std::io::{ErrorKind, Read, Write};
use std::net::{TcpStream, ToSocketAddrs};
use std::net::{SocketAddr, TcpStream, ToSocketAddrs};
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::{Duration, Instant};
@@ -182,27 +182,68 @@ fn tls_config() -> Result<Arc<rustls::ClientConfig>, HttpError> {
Ok(Arc::new(config))
}
/// Resolve a host name within `deadline`, checking the cancel flag while it runs. The system resolver blocks with no
/// timeout of its own, so it runs on a helper thread (which ends when the resolver returns); a lookup that takes too
/// long is given up on, and Cancel works during it (issue #1532).
fn lookup(
host: &str,
resolve: impl FnOnce() -> std::io::Result<Vec<SocketAddr>> + Send + 'static,
deadline: Instant,
limits: &Limits,
) -> Result<Vec<SocketAddr>, HttpError> {
if limits.cancel.load(Ordering::Relaxed) {
return Err(HttpError::Cancelled);
}
let (tx, rx) = std::sync::mpsc::channel();
std::thread::Builder::new()
.name("lc-fetch-lookup".into())
.spawn(move || {
let _ = tx.send(resolve());
})
.map_err(|e| HttpError::Connect(format!("{host}: {e}")))?;
loop {
if limits.cancel.load(Ordering::Relaxed) {
return Err(HttpError::Cancelled);
}
let left = deadline.saturating_duration_since(Instant::now());
if left.is_zero() {
return Err(HttpError::Connect(format!("{host}: the name lookup timed out")));
}
match rx.recv_timeout(left.min(TICK)) {
Ok(r) => return r.map_err(|e| HttpError::Connect(format!("{host}: {e}"))),
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {}
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => return Err(HttpError::Connect(format!("{host}: the name lookup failed"))),
}
}
}
/// Connect to the first of `addrs` that answers, all of them together within `deadline` (each used to get the whole
/// connect timeout of its own, issue #1532).
fn connect_any(host: &str, addrs: &[SocketAddr], deadline: Instant, limits: &Limits) -> Result<TcpStream, HttpError> {
let mut last = format!("{host}: no address");
for a in addrs {
if limits.cancel.load(Ordering::Relaxed) {
return Err(HttpError::Cancelled);
}
let left = deadline.saturating_duration_since(Instant::now());
if left.is_zero() {
return Err(HttpError::Connect(format!("{host}: connecting timed out")));
}
match TcpStream::connect_timeout(a, left) {
Ok(s) => return Ok(s),
Err(e) => last = format!("{host}: {e}"),
}
}
Err(HttpError::Connect(last))
}
impl Stream {
fn connect(url: &Url, limits: &Limits) -> Result<Stream, HttpError> {
let addrs: Vec<_> = (url.host.as_str(), url.port).to_socket_addrs().map_err(|e| HttpError::Connect(format!("{}: {e}", url.host)))?.collect();
if addrs.is_empty() {
return Err(HttpError::Connect(format!("{}: no address", url.host)));
}
let mut last = String::new();
let mut sock = None;
for a in addrs {
if limits.cancel.load(Ordering::Relaxed) {
return Err(HttpError::Cancelled);
}
match TcpStream::connect_timeout(&a, limits.connect) {
Ok(s) => {
sock = Some(s);
break;
}
Err(e) => last = format!("{}: {e}", url.host),
}
}
let sock = sock.ok_or(HttpError::Connect(last))?;
// the lookup and the connection attempts share one connect timeout
let deadline = Instant::now() + limits.connect;
let (host, port) = (url.host.clone(), url.port);
let addrs = lookup(&url.host, move || (host.as_str(), port).to_socket_addrs().map(Iterator::collect), deadline, limits)?;
let sock = connect_any(&url.host, &addrs, deadline, limits)?;
sock.set_read_timeout(Some(TICK)).map_err(|e| HttpError::Io(e.to_string()))?;
sock.set_write_timeout(Some(TICK)).map_err(|e| HttpError::Io(e.to_string()))?;
let _ = sock.set_nodelay(true);
@@ -512,6 +553,49 @@ pub fn get(url: &Url, headers: &[(&str, String)], limits: &Limits) -> Result<Res
Ok(Response { status, headers, conn, body })
}
#[cfg(test)]
mod tests_connect {
use super::*;
fn limits(cancel: &AtomicBool) -> Limits<'_> {
Limits { connect: Duration::from_millis(300), stall: Duration::from_secs(1), cancel }
}
/// A name lookup that hangs is given up on at the connect timeout, and Cancel stops it at once (issue #1532).
#[test]
fn a_hung_name_lookup_times_out_and_can_be_cancelled() {
let cancel = AtomicBool::new(false);
let slow = || {
std::thread::sleep(Duration::from_secs(10));
Ok(Vec::new())
};
let t = Instant::now();
let r = lookup("slow.example", slow, Instant::now() + Duration::from_millis(300), &limits(&cancel));
assert!(matches!(&r, Err(HttpError::Connect(m)) if m.contains("timed out")), "{r:?}");
assert!(t.elapsed() < Duration::from_secs(2), "{:?}", t.elapsed());
cancel.store(true, Ordering::Relaxed);
let t = Instant::now();
assert!(matches!(lookup("slow.example", slow, Instant::now() + Duration::from_secs(30), &limits(&cancel)), Err(HttpError::Cancelled)));
assert!(t.elapsed() < Duration::from_secs(1));
// a lookup that answers is used
let cancel = AtomicBool::new(false);
let local: SocketAddr = "127.0.0.1:9".parse().unwrap();
assert_eq!(lookup("local", move || Ok(vec![local]), Instant::now() + Duration::from_secs(5), &limits(&cancel)).unwrap(), [local]);
}
/// Several addresses that don't answer share one deadline instead of each getting the full timeout.
#[test]
fn addresses_share_one_connect_deadline() {
let cancel = AtomicBool::new(false);
// non-routable addresses: each either hangs until the deadline or fails at once, never longer
let addrs: Vec<SocketAddr> = (1..=5).map(|i| format!("10.255.255.{i}:81").parse().unwrap()).collect();
let t = Instant::now();
let r = connect_any("blackhole", &addrs, Instant::now() + Duration::from_millis(400), &limits(&cancel));
assert!(r.is_err());
assert!(t.elapsed() < Duration::from_millis(1500), "{:?}", t.elapsed());
}
}
#[cfg(test)]
mod tests {
use super::*;
+5
View File
@@ -2,6 +2,11 @@
## 2026-10-14
### Reliability
- Model downloads (AI denoise, faces, SAM) give up on a server name that doesn't resolve within the connect timeout,
and Cancel stops a download that is still looking the name up (issue #1532). The lookup used to block with no time
limit, and each of a server's addresses then got the whole connect timeout of its own.
### Hard-edged Heal and Clone spots
- A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity
changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged.