Repository navigation
Description
rig_https_banner.py burns its whole fetch budget polling a frozen counter after the verdict is already decided
Summary
Once check_body_complete has a framing-derived expectation and the body has stopped advancing, the truncation verdict is knowable. The poll loop does not use it: its only exit is body.ok, so a truncated fetch runs to FETCH_TIMEOUT and then waits out the close.
Measured on a real red run (U64E @ 48 MHz, FETCH_TIMEOUT=900, uci-onchip PRG 44e6c0dd605db65682131d6621fc9ad7b8b03cb0d22b0b241691abd3c35ed32a, HTTPS_HOST=en.wikipedia.org HTTPS_PATH=/wiki/Commodore_64):
http_body_totalclimbed to 299,123 B of a declared 754,413 B, then froze.- The rig polled the frozen counter for ~700 s / 44 consecutive polls, with the C64 sitting at the BASIC
ready.prompt. - Then
[close] 'CONNECTION CLOSED' NOT seen within 120s. - ~14 of 17 minutes of shared-device time spent on a verdict that was already decided.
At TURBO_MHZ=1 this is far worse: _SCALE=48 makes FETCH_TIMEOUT=14,400 s and INIT_WAIT=3,600 s, so a single truncated 1 MHz run holds the device for over five hours producing no information after the first minute.
Why this is worth fixing
This is a throughput problem, not a correctness one — the verdict is right, just late — so it is genuinely lower priority than anything that changes a result. Two things raise it above "nice to have":
- The device is shared across
c64-*projects and lanes queue onDeviceLock. The lock's heartbeat means a queued lane stalls rather than timing out, so this converts one lane's failed run into every other lane's wait. - http_get reports success (carry=0, HTTP 200) on a truncated body — silent data loss #211 is intermittent and its cause is unlocated, so red runs are not rare and will not be rare for a while. The rig is the instrument for diagnosing that, and an instrument that takes 17 minutes to say "stalled at 299,123" discourages the repeated runs the diagnosis needs.
The concern in the rig's own review brief — that a rig which fails spuriously gets muted — applies to a rig that is merely slow to fail.
The shape of a fix
An early break needs care, because "stopped advancing" is exactly what check_fetch_settled exists to be cautious about: a body still growing when the budget expires is INCONCLUSIVE, not TRUNCATED, and that distinction was added because a 300 s run had already cried wolf on a body that was still climbing.
So the early exit must not simply be "the counter did not move". A defensible shape:
- Track the last-advance timestamp the poll loop already needs for
check_fetch_settled. - Break early only when the body has been frozen for substantially longer than
STALL_GRACE— a separate, larger constant, so the settled check keeps its own meaning — and the framing gives a definite expectation (cl_valid, or chunked with a terminal chunk expected), and the socket state agrees the stream is done. - On an early break, report the same verdict text with an explicit note that the budget was not exhausted, so a reader can tell a "frozen for N s" verdict from a "budget expired" one.
Both constants deserve a measurement rather than a guess. The observed stall shape is a strong input: in the run above the freeze was abrupt and total — no trickle — which is consistent with #219's fast-expiry path, where an errored socket makes net_poll a 6-cycle RTS. A slow-trickle pathology has never been observed; if one exists, an early break sized off the abrupt shape would misfire on it, which is the argument for a generous constant rather than a tight one.
The 120 s close wait after a decided FAIL is separately worth revisiting: on a truncated run the close is not expected to arrive, so waiting the full window is pure cost.
Related
- http_get reports success (carry=0, HTTP 200) on a truncated body — silent data loss #211 — the truncation itself; cause unlocated.
- rig_https_banner.py cannot fail on a truncated body #210 — the rig could not fail on a truncated body at all; the oracle this issue is about was added there.
- The lease-poisoning note in
tools/uci/README.md: a red run reliably ends with the machine atready., i.e. the state the docstring warns about. Not caused by this issue, but the long tail makes the window larger.
Not claimed
No measurement of how much lock time this actually costs across a session, and no evidence that any lane has yet been meaningfully delayed by it — the mechanism is measured, the aggregate impact is not.
Activity
fix(rig): pin #226's rig wiring by value, re-check shadow after 'Q'
test(rig): pin #226's rig wiring by exact AST, refuse non-finite knobs
test(rig): execute the close wait, pin the 'Q' and the stall knobs (#226
test(rig): pin poll_until's budget with a non-10 s case (#226)
Merge pull request #232 from JC-000/fix/226-banner-early-stop