Fail dead-socket sends instead of reporting zero progress - #200
Merged
Conversation
send_raw() returned pos -- 0 when nothing was sent this call -- once the client was already marked CSTATE_DONE, and http_printv() advances its send loop by that return value: pos += 0 never reaches len, so a worker whose client disconnected mid-response spun forever at 100% CPU issuing send() SVCs (nucleus-heavy ENQ/DEQ via the errno lock). Enough of these and the whole pool is gone. The #159 escape never sees it because the spin is below serve_client(), inside the handler's output path. Fix both sides of the no-progress contract: - send_raw() now returns -1 on any hard send() failure, regardless of client state. A 0 return stays reserved for EWOULDBLOCK, which the static-file path (httpfile.c) relies on as its cooperative "no progress, retry next pass" signal -- that semantic is unchanged. - http_printv() sends through a new send_all() helper that treats 0 as "socket send buffer full": pause 100 ms (libc370 usleep) and retry, bounded at 10 seconds of consecutive zero progress, after which the client is marked done and the send fails. A stalled reader now costs a paced wait instead of a busy spin; a dead socket fails immediately, as does any zero-progress send once the client is past CSTATE_DONE. Fixes #199
This was referenced Aug 18, 2026
mgrossmann
added a commit
that referenced
this pull request
Aug 18, 2026
Verifying the shutdown-quiesce fix (#205) live turned up a Hercules platform bug that had been latent all along: X'75' SEND (tcpip.c) issues a blocking host send() on the emulated CPU thread, so a client that holds a connection open and stops reading fills the host send buffer, freezes a CP, and the watchdog reacts to the stalled CP by deliberately crashing the whole emulator. It killed the machine five times before we understood it, and it is not a test artifact -- a laptop closing or a VPN dropping without RST mid-response is the same traffic shape. The new document records the root cause with the gdb backtrace, the three preconditions that have to coincide, the exact client script, and RED/GREEN criteria, so the next person can re-verify after a Hercules update rather than rediscover it. It also explains why our earlier tests never hit it: aborting clients produce a reset (send fails immediately), slow-but-reading clients keep making progress, and the #199 spin executes billions of instructions, which looks healthy to the watchdog. The two findings are linked: until SEND stops blocking, a full send buffer never surfaces EWOULDBLOCK to the guest, so the stall budget from #200/#202 and the quiesce check from #205 are unreachable on this platform. With SEND patched, both fired live for the first time -- P HTTPD under continuous stall load now completes in 3 s instead of requiring C HTTPD. Cross-referenced from the shutdown-drain section in development.md, which covers the same discipline for module authors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #199
Problem
Two functions disagreed on the no-progress contract. Once a client was marked
CSTATE_DONE, a failingsend()insend_raw()(src/httpsend.c) took the barebreakand returnedpos— 0 when nothing was sent this call.http_printv()(src/httpprtv.c) advances its send loop by that return value and only checksrc < 0, sopos += 0never reacheslen: the worker spins forever at ~100% CPU issuingsend()SVCs against the dead socket. Wedged workers never return to the pool; accumulate a few and the server answers nothing (the mechanism behind mvslovers/mvsmf#217). The #159 escape never fires because the spin is belowserve_client(), inside the handler's output path.Fix
Both sides of the contract:
send_raw()returns-1on any hardsend()failure, regardless of client state — a dead socket never reports success-shaped progress. A0return stays reserved forEWOULDBLOCK: the static-file path (httpfile.c) relies on that as its cooperative "no progress, retry next pass" signal, so that semantic is deliberately unchanged.http_printv()sends through a newsend_all()helper (replacing the three inlinepos += rcloops, including both chunked-injection loops).rc == 0now means "socket send buffer full": pause 100 ms (usleep()from libc370) and retry, bounded at 10 s of consecutive zero progress, after which the client is marked done and the send fails. Once the client is pastCSTATE_DONE, a zero-progress send fails immediately. A stalled reader costs a paced wait instead of a busy spin; a dead socket fails at once.Verification
make modulesclean (cc370,-Wall -Werror),make testbuilds all 17 test modules,make test-host98 assertions pass.curls against/.dsrv?target=HTTPDon a test instance — before: workers wedge (D THREADSshows RUNNING with frozenDISPCNT, guest CP MIPS stays 70+ with no clients); after: MIPS returns to idle, all workers return to the pool, and a subsequent request answers normally.