Send a chunk completely or not at all

[mgrossmann]

Fixes #201

Problem

The chunked path of http_send() checked its three send_raw() calls — chunk header, data, trailing CRLF — only for a negative return, then returned len unconditionally. send_raw() legitimately returns partial progress: the sockets are non-blocking (FIONBIO), so a full TCP send buffer breaks it out on EWOULDBLOCK having written anywhere from 0 to len-1 bytes. The header had already announced len bytes, so the stream went short and the next chunk header — or the 0\r\n\r\n terminator from httpdone() — landed mid-chunk where the client expected data bytes. From there the framing is garbage. The caller was meanwhile told everything went out, so no layer could attempt recovery.

Fix

Header, data and trailer go through a new send_raw_all(), which completes the buffer or fails. A closed receive window is waited out with the same paced retry and no-progress budget http_printv() got in #200 — the two constants now live in httpd.h so they cannot drift apart. A dead socket still fails immediately.

When a chunk cannot be completed:

• framing is dropped (chunked = 0), so httpdone() cannot append a terminator that would tell the client a truncated response ended normally;
• keep-alive is cleared, on the reasoning httppars() already applies to an unread body — the connection must not carry a torn chunk into the next request;
• CSTATE_DONE is set on every failing return. This matters: http_send_file() (httpfile.c:100, :333) tests only for a positive return and ignores -1, so the client state is what actually stops it.

The non-chunked contract is deliberately untouched — a partial return stays visible there, which is what http_send_file() drives its state machine on.

Note: this path is reached by static files, not just CGIs

Worth recording, since it was checked rather than assumed. http_send_file() does run with chunked == 1, in two real cases (httpget.c:106-127): SSI files, which deliberately get no Content-Length, and UFS files where ufs_stat() fails or reports filesize == 0. For those, a slow reader now waits inside http_send() (bounded at 10 s) instead of returning to the state machine for another pass. That is the intended trade: a chunk cannot be made atomic any other way, and the previous behaviour on a slow reader was a busy pass-loop that #159's 1000-pass escape could cut off mid-response.

Verification

• make modules clean (cc370, -Wall -Werror), make test builds all 17 test modules, make test-host 98 assertions pass.
• No unit test: the logic hinges on send()/errno/socket state and is not DUAL-testable as written. Extracting the stall decision into a pure function (the httpbody.c / httpstat.c pattern) would make it testable and is worth doing if this area is touched again.
• Not yet run against a live system. The live check is a chunked response to a slow reader — e.g. an SSI page or a filesize == 0 UFS file fetched with curl --limit-rate 2k — which must arrive complete and correctly framed; curl reports a transfer error if any chunk is short.
• Not addressed here: a worker inside the paced retry does not check HTTPD_FLAG_QUIESCE/SHUTDOWN (same family as Shutdown crash: S33E on worker DETACH + recovery ESTAE runs C runtime under a torn-down CRT #122). That applies equally to the pause Fail dead-socket sends instead of reporting zero progress #200 already put in http_printv(), so it belongs in both places or neither — a follow-up, not this PR.

[mgrossmann]

Two things checked after opening this, both worth recording:

usleep() is safe in worker context. The paced retry here (and the one #200 already merged into http_printv()) calls libc370's usleep(), which is STIMER WAIT — an SVC that suspends a TCB. That is only acceptable if a worker owns its TCB rather than being multiplexed inside a shared one; otherwise one slow client would freeze the whole server for up to 10 s. Confirmed it does: @@ctcrtx.c:87 in libc370 creates every CTHREAD worker with ATTACH EP=CTHREAD,ECB=…, so each worker is a real MVS subtask and STIMER suspends only that worker. Note the existing STIMER WAIT at httpd.c:1208 is not the precedent it looks like — that one is in main(), on the main task.

Invariant now holds by construction (dfcfc7c). send_raw_all() sets CSTATE_DONE itself on every negative return instead of relying on send_raw() having done it: the MSG_SEND_UNDERFLOW guard also returns negative without touching the state, and since http_send_file() ignores -1 and stops on the client state alone, that gap would have been load bearing on a path that is otherwise unreachable.

Also opened #203 for the third item on #199's fix list, which never landed and had no ticket after #199 closed — the unchecked http_printf() cascades in /.dsrv. Not a hang any more since #200, just wasted work against a dead socket.