Model download has no network timeouts; a stalled server hangs the download thread forever #10

New Issue

hugo · 2026-07-25T09:39:28Z

hugo commented

2026-07-25 09:39:28 +00:00

Symptom

A model download that stalls (server accepts the connection then stops sending) hangs forever. The Cancel button in the UI does nothing, because the cancel flag is only checked between reads and the read never returns. For an 80 GiB artifact over a flaky link this is a realistic and unrecoverable state — the user has to quit the app.

Evidence

src/model/transfer.rs:234-244:

let agent: ureq::Agent = ureq::Agent::config_builder()
    .https_only(url.starts_with("https://"))
    .build()
    .into();

No timeout_global, timeout_connect, timeout_recv_response, or timeout_recv_body. ureq applies no deadline by default.

The cancel check is outside the blocking call:

loop {
    if cancel.load(Ordering::Relaxed) { return Ok(DownloadOutcome::Stopped); }   // :266
    let count = body.read(&mut buffer)...;                                        // :269  blocks forever

verify() at src/model/transfer.rs:186-196 has the same shape, but reads from a local file, so it is not affected.

Second, smaller problem on the same line: .https_only(url.starts_with("https://")) only enforces HTTPS when the URL is already HTTPS, which is a no-op. URLs are built from constants (src/model.rs:258), so nothing is exploitable today, but the guard reads as protection it does not provide. It should be .https_only(true).

How ds4 handles this

../ds4 gives every socket operation a named timeout constant — it is the house convention, not an afterthought:

ds4_web.c:30-31 — DS4_WEB_CONNECT_TIMEOUT_MS 3000, DS4_WEB_CDP_TIMEOUT_MS 20000
ds4_web.c:169 web_tcp_connect(host, port, timeout_ms, ...) and ds4_web.c:234 web_read_some(fd, buf, len, timeout_ms) — every read goes through a poll() with an explicit deadline; ds4_web.c:486-493 turns an expired deadline into a "websocket read timeout" error rather than a hang.
ds4_server.c:45 — DS4_SERVER_IO_TIMEOUT_SEC 10, applied to every accepted socket at ds4_server.c:12498-12503 via SO_RCVTIMEO/SO_SNDTIMEO.

Note for honesty: ds4's model downloader is download_model.sh, which shells out to curl -fL --progress-meter -C - (download_model.sh:335) and also sets no explicit timeout. But that runs in a terminal where a stalled transfer is visible in the progress meter and killable with Ctrl-C. DS4Server is a GUI where the same stall is invisible and the Cancel button is inert — so the missing timeout is materially worse here than in the reference.

Suggested fix

Configure the agent with an overall and a per-read deadline, following ds4's named-constant convention:

const DOWNLOAD_STALL_TIMEOUT: Duration = Duration::from_secs(30);

let agent: ureq::Agent = ureq::Agent::config_builder()
    .https_only(true)
    .timeout_connect(Some(Duration::from_secs(10)))
    .timeout_recv_response(Some(DOWNLOAD_STALL_TIMEOUT))
    .build()
    .into();

Check the ureq 3.x config surface for the body-read stall timeout name before writing it — the crate has renamed these across versions. A stalled read should surface as Err("Model download failed: ..."), which the existing resume logic already handles: the partial file is kept and download_url_to_partial will re-Range from where it stopped on the next attempt (src/model/transfer.rs:233-253).

Acceptance criteria

A download against a server that accepts and then stalls fails with a readable error within ~30 s instead of hanging.
After the failure the .part file is intact and a retry resumes from the existing offset (verified by the existing download_artifact resume tests in src/model/transfer.rs:420+).
Cancel during a healthy download still works.
.https_only(true) unconditionally.

## Symptom A model download that stalls (server accepts the connection then stops sending) hangs forever. The Cancel button in the UI does nothing, because the cancel flag is only checked *between* reads and the read never returns. For an 80 GiB artifact over a flaky link this is a realistic and unrecoverable state — the user has to quit the app. ## Evidence `src/model/transfer.rs:234-244`: ```rust let agent: ureq::Agent = ureq::Agent::config_builder() .https_only(url.starts_with("https://")) .build() .into(); ``` No `timeout_global`, `timeout_connect`, `timeout_recv_response`, or `timeout_recv_body`. `ureq` applies no deadline by default. The cancel check is outside the blocking call: ```rust loop { if cancel.load(Ordering::Relaxed) { return Ok(DownloadOutcome::Stopped); } // :266 let count = body.read(&mut buffer)...; // :269 blocks forever ``` `verify()` at `src/model/transfer.rs:186-196` has the same shape, but reads from a local file, so it is not affected. Second, smaller problem on the same line: `.https_only(url.starts_with("https://"))` only enforces HTTPS when the URL is *already* HTTPS, which is a no-op. URLs are built from constants (`src/model.rs:258`), so nothing is exploitable today, but the guard reads as protection it does not provide. It should be `.https_only(true)`. ## How ds4 handles this `../ds4` gives every socket operation a named timeout constant — it is the house convention, not an afterthought: - `ds4_web.c:30-31` — `DS4_WEB_CONNECT_TIMEOUT_MS 3000`, `DS4_WEB_CDP_TIMEOUT_MS 20000` - `ds4_web.c:169` `web_tcp_connect(host, port, timeout_ms, ...)` and `ds4_web.c:234` `web_read_some(fd, buf, len, timeout_ms)` — every read goes through a `poll()` with an explicit deadline; `ds4_web.c:486-493` turns an expired deadline into a `"websocket read timeout"` error rather than a hang. - `ds4_server.c:45` — `DS4_SERVER_IO_TIMEOUT_SEC 10`, applied to every accepted socket at `ds4_server.c:12498-12503` via `SO_RCVTIMEO`/`SO_SNDTIMEO`. Note for honesty: ds4's *model downloader* is `download_model.sh`, which shells out to `curl -fL --progress-meter -C -` (`download_model.sh:335`) and also sets no explicit timeout. But that runs in a terminal where a stalled transfer is visible in the progress meter and killable with Ctrl-C. DS4Server is a GUI where the same stall is invisible and the Cancel button is inert — so the missing timeout is materially worse here than in the reference. ## Suggested fix Configure the agent with an overall and a per-read deadline, following ds4's named-constant convention: ```rust const DOWNLOAD_STALL_TIMEOUT: Duration = Duration::from_secs(30); let agent: ureq::Agent = ureq::Agent::config_builder() .https_only(true) .timeout_connect(Some(Duration::from_secs(10))) .timeout_recv_response(Some(DOWNLOAD_STALL_TIMEOUT)) .build() .into(); ``` Check the `ureq` 3.x config surface for the body-read stall timeout name before writing it — the crate has renamed these across versions. A stalled read should surface as `Err("Model download failed: ...")`, which the existing resume logic already handles: the partial file is kept and `download_url_to_partial` will re-`Range` from where it stopped on the next attempt (`src/model/transfer.rs:233-253`). ## Acceptance criteria - A download against a server that accepts and then stalls fails with a readable error within ~30 s instead of hanging. - After the failure the `.part` file is intact and a retry resumes from the existing offset (verified by the existing `download_artifact` resume tests in `src/model/transfer.rs:420+`). - Cancel during a healthy download still works. - `.https_only(true)` unconditionally.

hugo added the bug label 2026-07-25 09:39:28 +00:00

hugo commented

2026-07-25 10:52:57 +00:00

Fixed in 429cbc7. Model downloads now enforce a 10 s connect timeout plus 30 s response/body stall timeouts, and production downloads are HTTPS-only. Added a loopback regression test proving a stalled transfer times out without damaging the resumable partial file. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

Fixed in 429cbc7. Model downloads now enforce a 10 s connect timeout plus 30 s response/body stall timeouts, and production downloads are HTTPS-only. Added a loopback regression test proving a stalled transfer times out without damaging the resumable partial file. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

hugo closed this issue

2026-07-25 10:53:01 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: hugo/DS4Server#10