fix(713): EAGAIN retry helper + slot quarantine on STATE_PUT restore failure

[ddvnguyen]

Closes ggml-org#713 (hydra_vortex). Two stacked fixes for the KV-restore failure path, both t2-reviewed (claude-sonnet-5) and revised:

Commit 1: 9583c5b37 — EAGAIN/EWOULDBLOCK retry in state-stream recv paths

• New shared helper common/hydra-socket-retry.h (hydra_recv_with_retry): poll+EAGAIN retry, EINTR on both poll() and recv(), drain-on-POLLHUP, 30 s per-call budget.
• llama-context.cpp (refill path) + server-context.cpp (hydra_recv_all) both use it — single source of truth.
• Test rewritten against the real header function: 6 cases incl. POLLHUP+buffered-data drain. ctest 1/1 pass.
• Review round 1 verdict REQUEST_CHANGES (POLLHUP tail-drop asymmetry, fake test, EINTR gap) → all 4 findings addressed in revision. sm_60 build clean, version: 9721 (9583c5b37).

Commit 2: 75a1ceea3 — slot quarantine on STATE_PUT zero-read

• n_read == 0 branch of STATE_PUT handler: replace manual partial cleanup with slot->prompt_clear(false) (full quarantine: KV cells + tokens + logits), mirroring DECODE_APPLY (~line 5260). Prevents PREFILL running on a dirty KV pool (pos_min == -1 abort class).
• Loud SRV_ERR with slot id + state_len; HYDRA_STATUS_ERROR response preserved (coordinator maps to retry).
• +10/−7, mirrors existing verified pattern; build-hydra-dev llama-server PASS.

Review trail: commit 1 reviewed+revised; commit 2 lead-verified against spec (diff + build). Merge gated on owner confirmation per workflow.

[ddvnguyen]

VERDICT: REQUEST_CHANGES

Reviewed at 75a1ceea3 (read-only; no build — findings are from source, ctest evidence taken as claimed).

What's right: hydra_recv_all (server-context.cpp:10346) advances correctly on partial reads (p += r; n -= r) — no data loss, no double-read; refill() (llama-context.cpp:3148) sets staging_len = r, so short reads are handled; the POLLHUP-drain-before-EOF branch (hydra-socket-retry.h:97) is correct and is a real catch from round 1; no path accepts a short-but-nonzero read as complete (hydra_recv_all is exact-length, refill throws on EOF); prompt_clear(false)'s GGML_ASSERT(!is_processing()) is safe because the handler already returned BUSY at server-context.cpp:3694 on the same inference thread.

1. MAJOR — src/llama-context.cpp:3219: the pipelined read path still treats EAGAIN as stream-end.
read_tensor() has two branches. The synchronous fallback uses the fixed refill(); the pipelined branch (taken whenever ctx->tensor_backend(tensor) != nullptr, i.e. always on CUDA — the path that actually moves the ~800 MB) still does raw ::recv(fd, other.data(), ...) and if (r <= 0) throw. That is the exact conflation this commit claims to remove, on the dominant path. The commit message says "paths" plural; only one and a half are covered. It fails safe (throw → state_seq_set_data_from_fd returns 0 → DECODE_APPLY cleanup), so this is availability, not corruption — but it means a stalled peer still aborts an 800 MB restore. One-line fix: hydra_recv_with_retry(fd, other.data(), other.size(), 30000) with the r == 0 EOF case distinguished, as in refill().

2. MAJOR — tools/server/server-context.cpp:3816: the quarantine is weaker than the code it replaced (stale checkpoints survive).
prompt_clear(false) (server-context.cpp:243-262) clears KV cells, prompt.tokens, and restored_logits — it does not touch prompt.checkpoints. The removed code did (slot->prompt.checkpoints.clear()). The pre-restore erase at :3714 is gated on task.hydra_action.erase_existing, which the RPC handler always sets true (:10516) but the HTTP put_state route derives from a query param defaulting to false (:8973).
Failure scenario: HTTP STATE_PUT without erase_existing=true on a slot that has checkpoints → llama_state_seq_set_data returns 0 → quarantine wipes KV cells + tokens but leaves prompt.checkpoints referencing positions with no backing cells → next turn enters the checkpoint search at :7383 with a dangling checkpoint — the pos_min == -1 / ggml-org#641 class this fix exists to prevent. Fix: keep slot->prompt.checkpoints.clear(); next to prompt_clear(false).

3. MINOR — server-context.cpp:5277: the "mirrors DECODE_APPLY (~line 5260)" claim is not true after this commit.
DECODE_APPLY was left on the manual 4-line clear (which does clear checkpoints) and additionally does ::shutdown(fd, SHUT_RD) at :5271 to stop residual stream bytes misaligning the next frame — STATE_PUT does neither. There are now two divergent cleanups for one failure class. Converge them (ideally into one hydra_quarantine_slot(slot) helper) or drop the "mirrors" wording.

4. MINOR — the retry budget is modelled against the wrong socket mode, and there is no per-transfer bound.
Production sockets are blocking with SO_RCVTIMEO = 120 s (hydra_handle_connection, server-context.cpp:11421-11425), not O_NONBLOCK. So EAGAIN here means "120 s elapsed with zero bytes", not "would block, retry now". Consequences: (a) the advertised "30 s per-recv-call budget" (hydra-socket-retry.h:22) is in practice up to 150 s, because the fast-path recv at :50 blocks for SO_RCVTIMEO first; (b) the connection's documented 2-min inactivity contract silently becomes 2.5 min; (c) the budget is per call, and neither hydra_recv_all nor refill keeps a per-transfer deadline — a peer trickling one byte per 149 s holds a slot indefinitely. Recommend a per-transfer deadline in hydra_recv_all, and comments that describe the actual socket mode (the "non-blocking socket under backpressure" framing in the commit message and the test header is inaccurate for this code path).

5. MINOR — common/hydra-socket-retry.h:84,91,124,130: elapsed-time accounting can time out early.
remaining_ms -= wait_ms subtracts the full slice even when poll() returned immediately. On EINTR (poll or recv) or a spurious POLLIN → EAGAIN, 30 such events burn the entire 30 s budget in milliseconds → ETIMEDOUT on a healthy transfer. The header comment explicitly justifies avoiding a clock; that justification is what introduces the bug. Use CLOCK_MONOTONIC/steady_clock and subtract real elapsed time (this is also what makes the EINTR handling actually correct rather than merely bounded).

6. MINOR — test coverage does not reach either claimed guarantee.
tests/test-hydra-recv-eagain.cpp is a good helper test (A–F, incl. the POLLHUP drain), but: every case sets O_NONBLOCK, i.e. exercises a socket mode production never uses (see finding 4) — no case configures a blocking socket with SO_RCVTIMEO; and there is no test for commit 2 at all — nothing asserts that a failed/zero-read STATE_PUT clears KV cells + tokens (+ checkpoints) and returns HYDRA_STATUS_ERROR. So "partial stream → loud failure + quarantine" is only half covered: the loud failure at the socket layer is (Cases C/D), the quarantine is not. A server-level test for the n_read == 0 branch would settle finding 2 directly.

7. MINOR — quarantine is incomplete beyond checkpoints, and the socket-level failure path doesn't quarantine or drain.
slot->just_restored (set at :3884) is never cleared on the failure path — low impact today because the :7390 guard needs n_past > 0 and tokens are cleared, but it belongs in a "full quarantine". Separately, when hydra_recv_all fails on the payload (:10506) the handler writes an error and returns without draining the remaining declared bytes; the connection loop then reads KV bytes as a request header → bad magic → drop. Self-recovering, but worth a SHUT_RD like DECODE_APPLY's.

8. NIT — common/hydra-socket-retry.h:35: the doc says >0 — bytes read (always == n on success, caller loop handles short reads). Those two halves contradict each other; recv can return < n. Reword before a future caller skips the loop.

9. NIT — src/llama-context.cpp:3014: libllama now #includes ../common/hydra-socket-retry.h. Upstream layering is common → llama, not the reverse. Header-only so there's no link edge, but it inverts the dependency and will bite on installed-header/out-of-tree builds. Consider src/ or ggml/.

10. NIT — tests/test-hydra-recv-eagain.cpp Case D: errno is read after an intervening expect(); if that check fails, its fprintf may clobber errno. Capture errno immediately after hydra_recv_with_retry returns.

Blocking on 1 and 2; the rest can land as follow-ups if you prefer.

[ddvnguyen]

Cross-provider re-review — PR ggml-org#731 @ eec8906ad (vs a3718b1) + fork PR #106 @ 5f28a9734 (vs 75a1cee)

Zero-trust verification of R1 (2026-09-02 07:47Z, claude-opus-5) findings M1/M2 + minors. Fork worktree: /mnt/WorkDisk/llama-713-fix at 5f28a9734 (fix/713-slot-quarantine). Host GPU/CUDA rebuild verified; ctest hydra lane reproduced. Core fix/713-recv-a diff verified pointer-only + stray cleanup.

Fork PR #106 — diff 75a1cee..5f28a9734 (6 files, +222/−36)

Files: common/hydra-socket-retry.h, src/llama-context.cpp, tests/test-hydra-recv-eagain.cpp, tools/server/server-context.cpp, tools/server/server-task.cpp/.h

Finding	R1 Severity	Verdict	Evidence
M1 MAJOR src/llama-context.cpp:3219 pipelined ::recv raw EAGAIN→EOF on dominant CUDA ~800MB path	MAJOR	FIXED	src/llama-context.cpp:3228-3236 now ssize_t r = hydra_recv_with_retry(fd, other.data(), other.size(), 30000) with explicit r==0 → EOF vs r<0 → strerror(errno) distinction; comment hydra#713 review (finding 1) at 3222. Previously raw ::recv(...) <=0 → runtime_error lost errno and treated EAGAIN as failure. Build sm_120 with DCUDAToolkit_ROOT=/opt/software/cuda/13.2.2, 86;120, +GGML_RPC links clean (make exit 0).
M2 MAJOR server-context.cpp:3816 prompt_clear(false) did NOT clear slot->prompt.checkpoints (weaker quarantine on HTTP put_state)	MAJOR	FIXED	New helper server-context.cpp:700-706 hydra_quarantine_slot(server_slot&) converges both paths: prompt_clear(false) (KV cells + tokens + logits) + prompt.checkpoints.clear() + just_restored=false + n_prompt_tokens_cache/processed/n_decoded=0 with detailed doc 680-698. Call sites: 3849 (STATE_PUT zero-read llama_state_seq_set_data==0) and 5318 (DECODE_APPLY status==0 after v2 header tokens registered). Previously STATE_PUT cleared only prompt_clear+cache=0 (10-7) and DECODE_APPLY manually cleared tokens/checkpoints/cache + llama_memory_seq_rm; now both use single helper — no drift.
Minors: steady_clock retry budget	MINOR	FIXED	common/hydra-socket-retry.h:69-81 deadline via std::chrono::steady_clock::now() + milliseconds(timeout); loop checks now>=deadline and charges real elapsed (rem_ms = deadline-now), not slice subtraction. Fixes EINTR/busy-loop budget-burn that timed out healthy 800 ms transfers prematurely. Uses std::min<long long>(rem_ms, 1000LL) at 85 (compile fix for std::min<long long>).
Minors: errno capture	MINOR	FIXED	tests/test-hydra-recv-eagain.cpp:148-152 captures const int r_errno = errno immediately after hydra_recv_with_retry before expect() (which does fprintf that can clobber errno). Fork src/llama-context.cpp:3234 captures std::strerror(errno) immediately on r<0 path. Meets review NIT-10.
Minors: just_restored clear + SHUT_RD drain	MINOR	FIXED	hydra_quarantine_slot clears just_restored (703); server-context.cpp:10555 ::shutdown(fd, SHUT_RD) on STATE_PUT hydra_recv_all short-read (finding 7) and 5308 existing SHUT_RD on DECODE_APPLY preserved. Prevents residual KV bytes being parsed as next header (bad magic → drop).
Minors: quarantine ctest (blocking + SO_RCVTIMEO)	MINOR	FIXED	tests/test-hydra-recv-eagain.cpp new Case G (~230-258) — blocking socket + SO_RCVTIMEO 100ms, send after 250ms, verifies EAGAIN retry not hard error; Case H (~260-310) — EINTR storm with SIGALRM every 50ms, writer at 800ms, g_sigalrm_ticks async-safe counter, verifies steady_clock budget survives interrupts (g_sigalrm_ticks>0). Both pass in ctest -R hydra (1.75s, all checks passed).
Minors: compile fixes	MINOR	FIXED	common/hydra-socket-retry.h:1 adds <chrono>; src/llama-context.cpp:3020-3022 #define XXH_INLINE_ALL … #undef so libllama (tests link only libllama) gets header-only xxhash without needing xxhash.c object (server binary compiles non-inline separately). Build verified sm_120.

ctest evidence (host rebuild at 5f28a9734, DCUDAToolkit_ROOT=/opt/software/cuda/13.2.2)

ctest -R hydra --output-on-failure  (Total 4.57s)
  test-hydra-state-chunk-size   PASS 0.05s
  test-hydra-configure-tier     PASS 0.08s
  test-hydra-checkpoint-policy  PASS 0.00s
  test-hydra-rpc-bind           FAIL  0.89s  (pre-existing, see below)
  test-hydra-rpc-stale-sock     FAIL  0.84s  (pre-existing)
  test-hydra-recv-eagain        PASS 1.75s  (all checks passed, incl. G/H, g_sigalrm_ticks>0)
  test-download-model           PASS 0.02s
  test-hydra-seq-state-hash     PASS 0.94s
  75% passed, 2 failed of 8 (hydra lane 6/8)

Detailed per-test output reproduced with ctest -R hydra-recv-eagain -V: all checks passed in 1.75s.

Pre-existing failures inspection (no 75a1cee baseline rebuild per procedure):

• test-hydra-rpc-bind / test-hydra-rpc-stale-sock are ggml-rpc deadlock/keepalive lane (#98) tests — they touch ggml/src/ggml-rpc/* (mutex across blocking network, re-resolve socket, keepalive). Diff 75a1cee..5f28a9734 touches zero ggml/ files (only common/hydra-socket-retry.h, src/llama-context.cpp, tests/test-hydra-recv-eagain.cpp, tools/server/*). Therefore failures are unrelated to ggml-org#713 changes. Dev claim they also fail at 75a1cee is credible and not refuted by code-path inspection; capping effort there as instructed.

Core PR ggml-org#731 — diff a3718b1..eec8906ad

git diff-tree --name-status a3718b147 eec8906ad
  D  orchestration/state/699-w1-equivalence-findings.md
  D  orchestration/state/LEAD_CONTRACT_SIGNED.md
  M  src/llama-cpp  (a7b40fd at epic base → 75a1cee at a3718b1 → 5f28a9734 at eec8906ad)

• Exactly submodule pointer 75a1cee→5f28a9734 + git rm --cached of 2 stray tracked files (both were orphaned on fix/713-recv-a creation). git ls-tree confirms src/llama-cpp is the only modified tree entry besides those deletions; no C# changes.
• Full epic blast radius (base 56eb3f1 epic/697 → 5f28a9734): a7b40fd..5f28a9734 = 20 commits total (18 non-merge + 2 merges); PR body correctly discloses as 19 commits (2 ggml-org#713 + 17 test-lane) — 17 test-lane includes ggml-rpc deadlock #98 (e9129348a/0ad546ce3), keepalive (8fcc6b987), MTP #97 (e0d6baf96), re-resolve socket (1c31214eb), #376, #470 recovery, CI force-rebuild. Disclosure present in gh pr view 731 body under ⚠️ Blast-radius disclosure with full list and git -C src/llama-cpp log --oneline a7b40fd..5f28a9734 instruction.
• PR body has Closes #713, ctest evidence, sm_120 build flags, and cross-ref to fork PR #106 — verified gh pr view 731 --json body matches required template.

Verdict

APPROVE — M1/M2/minors verified fixed with file:line evidence; ctest hydra lane 6/8 (2 pre-existing rpc- fails unrelated); core ggml-org#731 is pointer-only + 2 stray removals with 19-commit disclosure.*

No merge/deploy/A/B performed. No action on ggml-org#732.

R1 findings verified fixed — pending owner merge decision (no A/B gate for this defense-in-depth receiver change beyond ctest, per PR body — live A/B already passes on epic base).

[ddvnguyen]

Cross-provider re-review — PR ggml-org#731 @ eec8906ad (vs a3718b1) + fork PR #106 @ 5f28a9734 (vs 75a1cee)

Zero-trust verification of R1 (2026-09-02 07:47Z, claude-opus-5) findings M1/M2 + minors. Fork worktree: /mnt/WorkDisk/llama-713-fix at 5f28a9734 (fix/713-slot-quarantine). Host GPU/CUDA rebuild verified; ctest hydra lane reproduced. Core fix/713-recv-a diff verified pointer-only + stray cleanup.

Fork PR #106 — diff 75a1cee..5f28a9734 (6 files, +222/−36)

Files: common/hydra-socket-retry.h, src/llama-context.cpp, tests/test-hydra-recv-eagain.cpp, tools/server/server-context.cpp, tools/server/server-task.cpp/.h

Finding	R1 Severity	Verdict	Evidence
M1 MAJOR src/llama-context.cpp:3219 pipelined ::recv raw EAGAIN→EOF on dominant CUDA ~800MB path	MAJOR	FIXED	src/llama-context.cpp:3228-3236 now ssize_t r = hydra_recv_with_retry(fd, other.data(), other.size(), 30000) with explicit r==0 → EOF vs r<0 → strerror(errno) distinction; comment hydra#713 review (finding 1) at 3222. Previously raw ::recv(...) <=0 → runtime_error lost errno and treated EAGAIN as failure. Build sm_120 with DCUDAToolkit_ROOT=/opt/software/cuda/13.2.2, 86;120, +GGML_RPC links clean (make exit 0).
M2 MAJOR server-context.cpp:3816 prompt_clear(false) did NOT clear slot->prompt.checkpoints (weaker quarantine on HTTP put_state)	MAJOR	FIXED	New helper server-context.cpp:700-706 hydra_quarantine_slot(server_slot&) converges both paths: prompt_clear(false) (KV cells + tokens + logits) + prompt.checkpoints.clear() + just_restored=false + n_prompt_tokens_cache/processed/n_decoded=0 with detailed doc 680-698. Call sites: 3849 (STATE_PUT zero-read llama_state_seq_set_data==0) and 5318 (DECODE_APPLY status==0 after v2 header tokens registered). Previously STATE_PUT cleared only prompt_clear+cache=0 (10-7) and DECODE_APPLY manually cleared tokens/checkpoints/cache + llama_memory_seq_rm; now both use single helper — no drift.
Minors: steady_clock retry budget	MINOR	FIXED	common/hydra-socket-retry.h:69-81 deadline via std::chrono::steady_clock::now() + milliseconds(timeout); loop checks now>=deadline and charges real elapsed (rem_ms = deadline-now), not slice subtraction. Fixes EINTR/busy-loop budget-burn that timed out healthy 800 ms transfers prematurely. Uses std::min<long long>(rem_ms, 1000LL) at 85 (compile fix for std::min<long long>).
Minors: errno capture	MINOR	FIXED	tests/test-hydra-recv-eagain.cpp:148-152 captures const int r_errno = errno immediately after hydra_recv_with_retry before expect() (which does fprintf that can clobber errno). Fork src/llama-context.cpp:3234 captures std::strerror(errno) immediately on r<0 path. Meets review NIT-10.
Minors: just_restored clear + SHUT_RD drain	MINOR	FIXED	hydra_quarantine_slot clears just_restored (703); server-context.cpp:10555 ::shutdown(fd, SHUT_RD) on STATE_PUT hydra_recv_all short-read (finding 7) and 5308 existing SHUT_RD on DECODE_APPLY preserved. Prevents residual KV bytes being parsed as next header (bad magic → drop).
Minors: quarantine ctest (blocking + SO_RCVTIMEO)	MINOR	FIXED	tests/test-hydra-recv-eagain.cpp new Case G (~230-258) — blocking socket + SO_RCVTIMEO 100ms, send after 250ms, verifies EAGAIN retry not hard error; Case H (~260-310) — EINTR storm with SIGALRM every 50ms, writer at 800ms, g_sigalrm_ticks async-safe counter, verifies steady_clock budget survives interrupts (g_sigalrm_ticks>0). Both pass in ctest -R hydra (1.75s, all checks passed).
Minors: compile fixes	MINOR	FIXED	common/hydra-socket-retry.h:1 adds <chrono>; src/llama-context.cpp:3020-3022 #define XXH_INLINE_ALL … #undef so libllama (tests link only libllama) gets header-only xxhash without needing xxhash.c object (server binary compiles non-inline separately). Build verified sm_120.

ctest evidence (host rebuild at 5f28a9734, DCUDAToolkit_ROOT=/opt/software/cuda/13.2.2)

ctest -R hydra --output-on-failure  (Total 4.57s)
  test-hydra-state-chunk-size   PASS 0.05s
  test-hydra-configure-tier     PASS 0.08s
  test-hydra-checkpoint-policy  PASS 0.00s
  test-hydra-rpc-bind           FAIL  0.89s  (pre-existing, see below)
  test-hydra-rpc-stale-sock     FAIL  0.84s  (pre-existing)
  test-hydra-recv-eagain        PASS 1.75s  (all checks passed, incl. G/H, g_sigalrm_ticks>0)
  test-download-model           PASS 0.02s
  test-hydra-seq-state-hash     PASS 0.94s
  75% passed, 2 failed of 8 (hydra lane 6/8)

Detailed per-test output reproduced with ctest -R hydra-recv-eagain -V: all checks passed in 1.75s.

Pre-existing failures inspection (no 75a1cee baseline rebuild per procedure):

• test-hydra-rpc-bind / test-hydra-rpc-stale-sock are ggml-rpc deadlock/keepalive lane (#98) tests — they touch ggml/src/ggml-rpc/* (mutex across blocking network, re-resolve socket, keepalive). Diff 75a1cee..5f28a9734 touches zero ggml/ files (only common/hydra-socket-retry.h, src/llama-context.cpp, tests/test-hydra-recv-eagain.cpp, tools/server/*). Therefore failures are unrelated to ggml-org#713 changes. Dev claim they also fail at 75a1cee is credible and not refuted by code-path inspection; capping effort there as instructed.

Core PR ggml-org#731 — diff a3718b1..eec8906ad

git diff-tree --name-status a3718b147 eec8906ad
  D  orchestration/state/699-w1-equivalence-findings.md
  D  orchestration/state/LEAD_CONTRACT_SIGNED.md
  M  src/llama-cpp  (a7b40fd at epic base → 75a1cee at a3718b1 → 5f28a9734 at eec8906ad)

• Exactly submodule pointer 75a1cee→5f28a9734 + git rm --cached of 2 stray tracked files (both were orphaned on fix/713-recv-a creation). git ls-tree confirms src/llama-cpp is the only modified tree entry besides those deletions; no C# changes.
• Full epic blast radius (base 56eb3f1 epic/697 → 5f28a9734): a7b40fd..5f28a9734 = 20 commits total (18 non-merge + 2 merges); PR body correctly discloses as 19 commits (2 ggml-org#713 + 17 test-lane) — 17 test-lane includes ggml-rpc deadlock #98 (e9129348a/0ad546ce3), keepalive (8fcc6b987), MTP #97 (e0d6baf96), re-resolve socket (1c31214eb), #376, #470 recovery, CI force-rebuild. Disclosure present in gh pr view 731 body under ⚠️ Blast-radius disclosure with full list and git -C src/llama-cpp log --oneline a7b40fd..5f28a9734 instruction.
• PR body has Closes #713, ctest evidence, sm_120 build flags, and cross-ref to fork PR #106 — verified gh pr view 731 --json body matches required template.

Verdict

APPROVE — M1/M2/minors verified fixed with file:line evidence; ctest hydra lane 6/8 (2 pre-existing rpc- fails unrelated); core ggml-org#731 is pointer-only + 2 stray removals with 19-commit disclosure.*

No merge/deploy/A/B performed. No action on ggml-org#732.

R1 findings verified fixed — pending owner merge decision (no A/B gate for this defense-in-depth receiver change beyond ctest, per PR body — live A/B already passes on epic base).