From e65b97e23922ff0a9d45d981a1dea3f1b8968740 Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Wed, 14 Oct 2026 13:08:15 -0500 Subject: [PATCH 1/2] 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. --- crates/fetch/src/http.rs | 124 ++++++++++++++++++++++++++++++++------- docs/whats-new.md | 5 ++ 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/crates/fetch/src/http.rs b/crates/fetch/src/http.rs index c2b5bee1e..98eee913b 100644 --- a/crates/fetch/src/http.rs +++ b/crates/fetch/src/http.rs @@ -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, 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> + Send + 'static, + deadline: Instant, + limits: &Limits, +) -> Result, 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 { + 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 { - 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 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 = (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::*; diff --git a/docs/whats-new.md b/docs/whats-new.md index 1bf44c3f4..3ca956764 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -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. From fe7edd7b26b211892a6fa2353a9304a2642bba96 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 18 Oct 2026 09:21:34 -0400 Subject: [PATCH 2/2] Model downloads: the What's New note gets its own heading Co-Authored-By: Claude Opus 5.5 --- docs/whats-new.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/whats-new.md b/docs/whats-new.md index 403bbe2c3..2f02ffb91 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,11 @@ ## 2026-10-14 +### Model downloads time out on a name that doesn't resolve +- 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. + ### Lightroom radial masks on the right side - Radial masks brought over from Lightroom Classic (catalog import, XMP sidecars, edits inside DNGs and presets) now apply their effect inside the circle, as Lightroom does, instead of outside it (issue #1815). A mask inverted in @@ -152,9 +157,6 @@ ⇧-click and the line tapers from the old values to the new ones. The whole line is one undo step. `mask.brushStroke {connect: true}` does the same for agents. -- 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.