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

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};
@@ -185,27 +185,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();
+        // the lookup and the connection attempts share one connect timeout
-        if addrs.is_empty() {
+        let deadline = Instant::now() + limits.connect;
-            return Err(HttpError::Connect(format!("{}: no address", url.host)));
+        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 mut last = String::new();
+        let sock = connect_any(&url.host, &addrs, deadline, limits)?;
-        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))?;
         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);
@@ -515,6 +556,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::*;

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