HTTP endpoint spawns unbounded threads and has no total request or write deadline #11

New Issue

hugo · 2026-07-25T09:40:22Z

hugo commented

2026-07-25 09:40:22 +00:00

Symptom

The local HTTP endpoint spawns one unbounded OS thread per accepted connection, and each connection can hold that thread indefinitely while buffering up to 64 MiB. Any local process can exhaust threads and memory. Additionally, a slow client that stops reading an SSE stream blocks its writer thread while the runtime's unbounded mpsc channel buffers the entire generation in RAM.

The server binds 127.0.0.1 only, so this is "any local process can wedge the endpoint", not remote. It still matters: a wedged endpoint looks like an app bug to the user.

Evidence

Unbounded threads — src/server.rs:256-274:

Ok((stream, _)) => {
    let state = Arc::clone(&state);
    let _ = thread::Builder::new()
        .name("http-request".into())
        .spawn(move || handle(stream, &state));
}

No counter, no cap, and the Result is discarded so a failed spawn silently drops the connection.

No total request deadline — src/server.rs:277 sets set_read_timeout(Some(Duration::from_secs(30))), but that is a per-read timeout. A client sending one byte every 29 s holds the thread for hours while read_request (src/server/http.rs:77-90) loops. Combined with unbounded spawning, that is a classic slowloris.

No write timeout — set_write_timeout is never called. A client that opens an SSE stream and stops reading blocks the writer in send_sse, while the runtime thread keeps producing into the unbounded mpsc::channel created at src/runtime.rs:75. Memory grows for the whole generation.

Minor, same function — src/server/http.rs:78 checks bytes.len() >= MAX_HEADER_BYTES at the top of the loop, so headers can overshoot the limit by one 4 KiB chunk. Cosmetic; note it while you are in there.

How ds4 handles this

../ds4/ds4_server.c is where src/server/http.rs was ported from — read_http_request at ds4_server.c:12235 has the same 64 KiB/64 MiB limits, the same 4096/8192 chunk sizes, and the same \r\n\r\n-then-\n\n header-end fallback. What did not get ported is the socket configuration around it:

ds4_server.c:12498-12503 configure_client_socket() sets SO_RCVTIMEO and SO_SNDTIMEO to DS4_SERVER_IO_TIMEOUT_SEC (10 s) on every accepted fd, before the request is read. DS4Server sets only a read timeout, and at 30 s.
ds4_server.c:12506-12513 set_client_socket_nonblocking() — and its comment states the design rule directly:

"The inference worker writes streaming responses itself. Once a request is queued, a blocked socket would block every other request too, so slow clients are failed instead of back-pressuring the model session."

That is precisely the failure mode DS4Server currently has. ds4 makes the socket non-blocking and lets send_all() (ds4_server.c:4473) fail the client after a 2 s poll(POLLOUT) stall deadline rather than let one slow reader stall the engine.
ds4_server.c:12491 listen(fd, 128) bounds the accept backlog, and the server tracks live clients under a mutex (s.clients++ at :13094, clients_cv broadcast at :12365) so shutdown can drain them — ds4_server.c:13127 while (s.clients > 0) pthread_cond_wait(&s.clients_cv, &s.mu);.
Concurrency is bounded by design: ds4_server.c:12942 slot_count = cfg.batched_sessions > 0 ? cfg.batched_sessions : 1, a fixed pool of worker threads created at :13037.

Suggested fix

Three independent, small changes; do them together:

Cap concurrent connections. Add an AtomicUsize to State, increment on accept, decrement in a guard on the request thread. Past a limit (64 is plenty for a localhost endpoint), reply 503 and close instead of spawning. Keeps the existing thread-per-connection shape — no async runtime needed.
Total deadline in read_request. Take an Instant at entry; abort with "HTTP request timed out" once the header+body read exceeds, say, 30 s in aggregate. Keep the per-read timeout as well.
Set a write timeout alongside the read timeout at src/server.rs:277, so a non-reading SSE client fails instead of blocking. Note this interacts with issue "Client disconnect does not cancel generation" — with the Drop-based cancel from that issue, a write timeout becomes a clean end-to-end abort.

Consider matching ds4's 10 s figure rather than inventing new ones, and defining them as named constants next to MAX_HEADER_BYTES in src/server.rs:42.

Acceptance criteria

Opening 200 concurrent connections does not create 200 threads; excess connections get a prompt 503 or close.
A client that sends GET /v1/models HTTP/1.1\r\n and then nothing is dropped within the deadline instead of holding a thread.
An SSE client that connects and never reads causes the request to fail rather than growing process RSS for the length of a generation.
Normal streaming and non-streaming requests are unaffected; existing src/server.rs tests still pass.

## Symptom The local HTTP endpoint spawns one unbounded OS thread per accepted connection, and each connection can hold that thread indefinitely while buffering up to 64 MiB. Any local process can exhaust threads and memory. Additionally, a slow client that stops *reading* an SSE stream blocks its writer thread while the runtime's unbounded `mpsc` channel buffers the entire generation in RAM. The server binds `127.0.0.1` only, so this is "any local process can wedge the endpoint", not remote. It still matters: a wedged endpoint looks like an app bug to the user. ## Evidence **Unbounded threads** — `src/server.rs:256-274`: ```rust Ok((stream, _)) => { let state = Arc::clone(&state); let _ = thread::Builder::new() .name("http-request".into()) .spawn(move || handle(stream, &state)); } ``` No counter, no cap, and the `Result` is discarded so a failed spawn silently drops the connection. **No total request deadline** — `src/server.rs:277` sets `set_read_timeout(Some(Duration::from_secs(30)))`, but that is a per-`read` timeout. A client sending one byte every 29 s holds the thread for hours while `read_request` (`src/server/http.rs:77-90`) loops. Combined with unbounded spawning, that is a classic slowloris. **No write timeout** — `set_write_timeout` is never called. A client that opens an SSE stream and stops reading blocks the writer in `send_sse`, while the runtime thread keeps producing into the unbounded `mpsc::channel` created at `src/runtime.rs:75`. Memory grows for the whole generation. **Minor, same function** — `src/server/http.rs:78` checks `bytes.len() >= MAX_HEADER_BYTES` at the top of the loop, so headers can overshoot the limit by one 4 KiB chunk. Cosmetic; note it while you are in there. ## How ds4 handles this `../ds4/ds4_server.c` is where `src/server/http.rs` was ported from — `read_http_request` at `ds4_server.c:12235` has the same 64 KiB/64 MiB limits, the same 4096/8192 chunk sizes, and the same `\r\n\r\n`-then-`\n\n` header-end fallback. What did **not** get ported is the socket configuration around it: - `ds4_server.c:12498-12503` `configure_client_socket()` sets `SO_RCVTIMEO` **and** `SO_SNDTIMEO` to `DS4_SERVER_IO_TIMEOUT_SEC` (10 s) on every accepted fd, before the request is read. DS4Server sets only a read timeout, and at 30 s. - `ds4_server.c:12506-12513` `set_client_socket_nonblocking()` — and its comment states the design rule directly: > "The inference worker writes streaming responses itself. Once a request is queued, a blocked socket would block every other request too, so slow clients are failed instead of back-pressuring the model session." That is precisely the failure mode DS4Server currently has. ds4 makes the socket non-blocking and lets `send_all()` (`ds4_server.c:4473`) fail the client after a 2 s `poll(POLLOUT)` stall deadline rather than let one slow reader stall the engine. - `ds4_server.c:12491` `listen(fd, 128)` bounds the accept backlog, and the server tracks live clients under a mutex (`s.clients++` at `:13094`, `clients_cv` broadcast at `:12365`) so shutdown can drain them — `ds4_server.c:13127` `while (s.clients > 0) pthread_cond_wait(&s.clients_cv, &s.mu);`. - Concurrency is bounded by design: `ds4_server.c:12942` `slot_count = cfg.batched_sessions > 0 ? cfg.batched_sessions : 1`, a fixed pool of worker threads created at `:13037`. ## Suggested fix Three independent, small changes; do them together: 1. **Cap concurrent connections.** Add an `AtomicUsize` to `State`, increment on accept, decrement in a guard on the request thread. Past a limit (64 is plenty for a localhost endpoint), reply `503` and close instead of spawning. Keeps the existing thread-per-connection shape — no async runtime needed. 2. **Total deadline in `read_request`.** Take an `Instant` at entry; abort with `"HTTP request timed out"` once the header+body read exceeds, say, 30 s in aggregate. Keep the per-read timeout as well. 3. **Set a write timeout** alongside the read timeout at `src/server.rs:277`, so a non-reading SSE client fails instead of blocking. Note this interacts with issue "Client disconnect does not cancel generation" — with the `Drop`-based cancel from that issue, a write timeout becomes a clean end-to-end abort. Consider matching ds4's 10 s figure rather than inventing new ones, and defining them as named constants next to `MAX_HEADER_BYTES` in `src/server.rs:42`. ## Acceptance criteria - Opening 200 concurrent connections does not create 200 threads; excess connections get a prompt `503` or close. - A client that sends `GET /v1/models HTTP/1.1\r\n` and then nothing is dropped within the deadline instead of holding a thread. - An SSE client that connects and never reads causes the request to fail rather than growing process RSS for the length of a generation. - Normal streaming and non-streaming requests are unaffected; existing `src/server.rs` tests still pass.

hugo added the bug label 2026-07-25 09:40:22 +00:00

hugo commented

2026-07-25 10:56:38 +00:00

Fixed in 9040070. The local endpoint now caps concurrent request threads at 64, releases slots structurally, enforces a 30 s total request deadline with 10 s per-I/O and write timeouts, reports thread-spawn failures, and enforces the header limit exactly. Added regression tests for slot saturation/release and slow-client deadlines. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

Fixed in 9040070. The local endpoint now caps concurrent request threads at 64, releases slots structurally, enforces a 30 s total request deadline with 10 s per-I/O and write timeouts, reports thread-spawn failures, and enforces the header limit exactly. Added regression tests for slot saturation/release and slow-client deadlines. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

hugo closed this issue

2026-07-25 10:56:44 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: hugo/DS4Server#11