HTTP server and lwIP under pressure (EPIC-13) - #14
Merged
Merged
Conversation
Three bugs kept a PUT larger than a couple of hundred kilobytes from ever completing. The idle sweeper closed live transfers. srv_poll_cb closed every connection but the debug stream on its first poll tick, and lwIP restarts a pcb's poll timer only when the peer acknowledges data the server sent -- during an upload the server sends nothing, so the timer ran however fast the data arrived. Connections now carry a last-activity stamp, set on accept and in the receive and sent callbacks, and are closed only after 20 s with no traffic either way. The HTTP_POLL_INTERVAL comment claimed 16 s per tick; it is 4 s. Every upload silently lost the body bytes that shared the first TCP segment with the headers. The header branch copied the segment into c->hdr clamped to 1024 bytes but called tcp_recved() for the whole pbuf, so with curl's 1460-byte first segment exactly 437 bytes were dropped while TCP told the client they had arrived -- the client had nothing to retransmit and the upload could never reach Content-Length. The body bytes now stay in the pbuf: the header branch hands (pbuf, offset) to parse_and_dispatch, which passes them to upload_drain_pbuf(), adv_load_drain_pbuf() or straight into c->body. This also fixes the Runner advanced-load body and any JSON body behind headers approaching 1 KB. A 4 MB upload then hit the watchdog: extending a multi-megabyte file makes FatFs walk the cluster chain and a single f_write outlasts the 8 s watchdog. upload_drain_pbuf() feeds it either side of each write and names the phase, as the GEMDRIVE write path does. On the error path, conn_close is split into conn_release_resources (FatFs handles, the partial-upload unlink, the body-stream lock) and the pcb teardown, and srv_err_cb calls the release. It used to drop the slot only, leaving the body lock set so every later transfer answered 503 busy until a reset. Verified on hardware, debug and release: 64 KB, 256 KB and 4 MB uploads return 200/201 and cmp identical after a round trip, a 4 MB download takes 15 s, 10 aborted uploads leave no partial file and the next upload returns 201, and smoke.py passes 8/8 on debug and 7/7 on release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
With DEVOPS_LWIP_STATS on a debug build, ran a 4 MB upload, a 4 MB download, a listing, a 40-request burst, two concurrent clients and 20 aborted transfers. No pool reported a single allocation failure, and only one was under real pressure. tcp_pcb measured 4 of 4 and stayed there between requests: the server closes every connection itself, so each request leaves a pcb in TIME_WAIT for 2 x TCP_MSL, and lwIP's default MSL of 60 s means 2 minutes per request. Nothing failed only because tcp_alloc evicts the oldest TIME_WAIT pcb when the pool is empty. TCP_MSL drops to 10 s, as in Booster, and the pool grows to 6 -- the 2 HTTP connections plus the debug stream plus headroom. Re-measured: 25 s after the same burst the pool is back to 1 of 6, where before it sat full indefinitely. tcp_seg peaked at 9 of 16 and pbuf_pool at 2 of 12, so both keep their size; each now carries the measurement that justifies it. pbuf_pool is the receive path -- the cyw43 driver allocates from it and in poll mode frees each pbuf before taking the next -- and a pool that runs dry there drops packets off the air, so its headroom stays. The two extra pcbs cost 312 bytes of .bss, which comes off the heap ceiling: total 46,224, minimum free 24,472, stack high water 2,544 of 16,384. Inside EPIC-12's budget. smoke.py passes 7/7 on release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
Audited every raw-TCP callback against the pinned lwIP (2.2.1, not the 2.1 this story assumed), reading core/tcp.c, tcp_out.c and tcp_in.c rather than relying on the API docs. tcp_recved() could run on a closed pcb. Both streaming branches of srv_recv_cb called a response writer -- which closes the connection on a FatFs error or a Runner chunk timeout -- and then accounted for the segment. tcp_close() on a pcb whose window has not been returned sends an RST, purges the pcb and unlinks it from the active list before returning ERR_OK, so that tcp_recved() was touching a pcb already queued to be freed. It is now done once at the top of srv_recv_cb, before anything can close. The adv-load branch also continued into adv_load_finish_ok() after a failed dispatch, which re-fired the failed chunk and wrote a second response onto a connection that had already sent one; it now honours the drain's return value. send_buffered treated ERR_MEM as fatal and dropped the response. lwIP queues nothing on ERR_MEM and expects the application to wait for acks and try again, so a client lost a reply it was still waiting for. The write is split out as send_buffered_try: ERR_MEM leaves resp_queued false, srv_poll_cb retries every 4 s, and the idle sweep gives up after 20 s. stream_send_chunk wrote a chunk as three tcp_writes with no send-buffer check on the debug-log path, so a failure after the first put a chunk-size line on the wire with no body behind it. It now refuses to start a chunk the send buffer cannot hold whole. Two contract violations that are unreachable today but wrong next to correct code: srv_recv_cb's NULL-arg path freed the pbuf and returned ERR_VAL, which makes lwIP keep and re-deliver that pbuf (it now aborts and returns ERR_ABRT), and conn_close's tcp_abort fallback left the callback returning ERR_OK after an abort (it now reports through conn_takeAborted()). An empty-body download set HC_DRAINING without stream_body_done, so srv_sent_cb never closed it and a zero-length GET held one of the two connection slots for 20 s. It completes in 19 ms now. Verified on hardware, both builds: 4 MB round trips cmp identical, error responses unchanged, 15 listings against a concurrent download, 20 aborted transfers with no pool errors, smoke.py 8/8 debug and 7/7 release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
stream_download_drive returns when tcp_write reports ERR_MEM and waits for a sent callback to pump the rest. That callback only arrives when the peer acknowledges something, so a writer that stopped with nothing unacknowledged -- ERR_MEM from an exhausted pool rather than a full window -- waited for an event that could not come, and the transfer sat there until the idle sweep closed it. srv_poll_cb now drives HC_STREAM_DOWNLOAD and HC_STREAM_LISTING as well as retrying a pending buffered response, then falls through to the idle check so a peer that really has gone away is still swept. The other two halves of this story landed with STORY-03: stream_send_chunk will not start a chunk the send buffer cannot hold whole, and send_buffered retries instead of dropping the response. Tested with EPIC-11's heap-hold hook at 5,112 bytes free, where the heap minimum reached 480 bytes and lwIP reported 1,595 failed allocations: error responses, listings, and 256 KB and 4 MB downloads all completed intact, a 4 MB upload returned 201, and service recovered when the hold was released. The same run on a458ac2 behaves the same, so that scenario does not discriminate between the builds; these are contract fixes read out of the lwIP source, not a reproduced failure. What does exercise the path is a slow reader: 256 KB to a client reading at 5 KB/s now arrives complete with a matching sha256 in 53 s, with the heap held at 5 KB free as well. In EPIC-09 that client was cut off after 8.4 s and 39,420 bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
FF_FS_LOCK counts open files and open non-root directories, and it is shared by everything on the card. The worst case this firmware can reach is 28: GEMDRIVE's 8 open files, its 16 searches (each DTA slot keeps a DIR open between Fsfirst and Fsnext), and 2 HTTP connections holding a FIL and a DIR each. The table was 8, so the ST on its own could exhaust it and starve any HTTP transfer, or the reverse. Sized to 28, with the arithmetic written next to it. Each entry is a 16-byte FILESEM, so the heap ceiling drops by 320 bytes, from 46,224 to 45,904 -- measured, and matching the prediction. Capping searches was the alternative, rejected because the cap would be enforced against a limit the ST cannot see. FR_TOO_MANY_OPEN_FILES was previously reported as whatever each handler's generic failure happened to be: Fopen said "file not found", Fcreate and Fsfirst said "path not found", and the HTTP server called it a disk error. It is now ENHNDL (-35) on all three GEMDOS paths and 503 too_many_open_files over HTTP, next to the existing 503 insufficient_memory. Both are retry-later conditions rather than faults. The HTTP half is verified on hardware: a 4 MB download with 10 listings running against it, all 200 and cmp identical, smoke.py 8/8. The ST half -- several desktop windows open during a copy and a transfer -- still needs the Atari. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
Six HTTP handlers block inside srv_recv_cb while the ST answers -- 10 s for a Runner load, 5 s for an unload, 1 s for an advanced-load chunk and for each of the three meminfo waits. Nothing else runs meanwhile, so for the length of the wait the SELECT button did nothing and queued console bytes stayed in the ring. All six had the same three lines, so they now share one http_spinTick() that also polls SELECT and drains USB CDC. The watchdog feed and the HTTP_WAIT phase were already there from EPIC-09 STORY-06. smoke.py 8/8 on debug. The check that actually enters a spin-wait -- SELECT pressed during a 10 s runner load -- needs the ST in Runner mode and is still open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
http_server_deinit closes every live connection, but it is not running inside an lwIP callback, so there is nobody to return ERR_ABRT. Leaving the flag set would make the next callback report an abort that never happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
…TORY-08) emul_resetRunnerSession() runs on the Runner's HELLO, which means the ST has cold booted and owns nothing. It cleared busy, cwd and the errnos but left runnerPendingBasepage set, so the RP stayed certain a program was still loaded across an ST reboot: every later load answered 409 program_already_loaded, and the unload that would have cleared it answered 422, because the ST rightly refuses to Mfree a basepage from a previous life (mfree_result=-40, EIMBA). Nothing the API offered could recover it -- one crashed program poisoned the Runner until the RP itself was rebooted. The session reset now clears the basepage mirror and the load errno with everything else. Verified on hardware: after an ST reset the API reports loaded_basepage null and loads work again. Found while soaking the Runner for STORY-08. The soak itself is still blocked by a separate defect that is not ours: exec takes the Atari down about one time in three, usually cold-booting it (which the RP now recovers from by itself) and occasionally wedging it until a physical reset. Written up in the story. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016emgLTZm7w3qZsRbukWwE4
diegoparrilla
added a commit
that referenced
this pull request
Sep 17, 2026
…ssure HTTP server and lwIP under pressure (EPIC-13)
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
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.
Makes the Remote API survive the traffic it is actually given. The epic's premise
understated the problem: before this branch, no HTTP upload much above 200 KB could
complete at all.
What was wrong
Three bugs stacked on each other, and a 256 KB
PUThit all three:srv_poll_cbclosed every connection but thedebug stream on its first tick, and lwIP restarts a pcb's poll timer only when the peer
acknowledges data the server sent — during an upload the server sends nothing, so the
timer ran however fast data arrived. Connections now carry a last-activity stamp and are
closed only after 20 s with no traffic either way.
a 1024-byte buffer but called
tcp_recved()for the whole 1460-byte pbuf, so the bodybytes sharing that segment with the headers were dropped and acknowledged. The client
had nothing to retransmit and the upload could never reach
Content-Length. Body bytesnow stay in the pbuf and are handed to the drain functions by offset.
the cluster chain, and a single
f_writeoutlasts the 8 s watchdog. The upload drain nowfeeds it either side of each write and names the phase.
The rest
srv_err_cbonly freed the slot, leaking the bodylock — every later transfer answered
503 busyuntil a reset — plus FatFs handles and thepartial file.
conn_closeis split so the error path releases everything.tcp_recved()could run on a pcbconn_closehad just closed;
send_bufferedtreatedERR_MEMas fatal and dropped the response insteadof retrying;
stream_send_chunkcould put a chunk-size line on the wire with no body. Everyrule was read out of the pinned lwIP source, not from the API docs.
tcp_pcbmeasured 4 of 4 and stayed there,because at lwIP's default 60 s MSL every request holds a pcb in TIME_WAIT for 2 minutes.
TCP_MSLdrops to 10 s and the pool grows to 6; re-measured, it now drains to 1 of 6 in 25 s.ERR_MEMwith nothingunacknowledged waited for a
sentcallback that could never come.srv_poll_cbdrives it.FF_FS_LOCKwas 8 against a worst case of 28 shared withGEMDRIVE, and
FR_TOO_MANY_OPEN_FILESwas reported as "file not found". Now 28, mapped toENHNDLand503 too_many_open_files.were dead for the whole wait.
Verified on hardware
Both builds, every claim measured rather than assumed:
PUTPUTcmpidenticalGETcmpidentical503until resetevery download sha256-verified, free heap back to its starting value.
failures): error responses, listings and 4 MB transfers all completed intact.
FR_TOO_MANY_OPEN_FILES; SELECT pressed 3.4 s into a 10 srunner loadreset the device, whichis only reachable through the new spin-wait poll.
smoke.py: 8/8 debug, 7/7 release.Cost
Heap ceiling drops 320 bytes for the lock table and 312 for the two extra pcbs; stack high water
2,580 of 16,384 across the soak.
Not included
STORY-08's Runner half is still open:
exectakes the Atari down about one time in three, whichblocks soaking the Runner. That is a Runner/m68k defect, not something this branch touched — the
HTTP server answered correctly throughout. One fix for it did land here: the session reset now
clears the loaded-program mirror on the Runner's HELLO, without which one crashed program left the
RP answering
409to every load until it was rebooted.