fix(tcp): drain send_buf on ACK — unstick sustained exchange (bidir_multi hang)#113
Closed
gspivey wants to merge 1 commit into
Closed
fix(tcp): drain send_buf on ACK — unstick sustained exchange (bidir_multi hang)#113gspivey wants to merge 1 commit into
gspivey wants to merge 1 commit into
Conversation
…xchange) handle_established's ACK path pruned the retransmit queue but never drained the acknowledged bytes from the front of send_buf, nor cleared has_unacked_data. After the first send+ACK, send_buf still held the stale acked bytes while already_sent_offset (derived from the now-empty retransmit queue) reset to 0 — so the next small write was misclassified as unsent behind Nagle (has_unacked_data==true, len<MSS) and never transmitted. The connection stalled after the first exchange. This is the sustained-exchange (bidir_multi) hang the TCP smoke tiers isolated on real hardware (#111 Graviton: single round-trip PASS, 20-on-one-connection HANG). Fix: on cumulative ACK, drain acked bytes off the front of send_buf, rebase the surviving retransmit-entry offsets, and clear has_unacked_data when nothing is in flight (snd_una==snd_nxt). Offline regression tests/loopback_stall_repro.rs wires client<->server engines through in-memory queues and drives 20 sequential 64B echoes on one connection: FAILS before (stalls at exchange 2), PASSES after. 221 unit + all property/ integration tests green. Follow-up: the same send_buf drain is missing in handle_fin_wait_1 / handle_close_wait (teardown states — a send_buf leak, not a stall). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Synthetic Performance Results — Graviton (run)Commit: ✅ synthetic UDP socket bound to 10.0.0.1:9000 (MAC: 02:00:00:00:00:01) Synthetic UDP Performance ResultsMeasures framework overhead: sync IPv4 Baseline
IPv6
IPv6 vs IPv4 Comparison (sync path)
IPv4 avg sync/async ratio: 1.0x, worst: 1.1x | IPv6 vs IPv4 worst ratio: 1.32x (OK)
|
Synthetic Performance Results (run)Commit: ✅ synthetic UDP socket bound to 10.0.0.1:9000 (MAC: 02:00:00:00:00:01) Synthetic UDP Performance ResultsMeasures framework overhead: sync IPv4 Baseline
IPv6
IPv6 vs IPv4 Comparison (sync path)
IPv4 avg sync/async ratio: 0.9x, worst: 1.1x | IPv6 vs IPv4 worst ratio: 1.30x (OK)
|
[CI] Stage: DeployInfrastructure ready.
|
[CI] Stage: DeployInfrastructure ready.
|
Owner
Author
|
Consolidated into #111. The send_buf-drain engine fix + loopback repro are now on agent/tcp-smoke-tiers. |
[CI] Stage: SummaryAll tests PASSED. ARP seeding: kernel /proc/net/arp (automatic)
|
✅ Integration Tests Passed — Graviton (run)Branch: Test Results
Application Logs (last 20 lines)receiver-echo-server.log sender-echo-server.log sender-test-client.log |
[CI] Stage: SummaryAll tests PASSED. ARP seeding: kernel /proc/net/arp (automatic)
|
✅ Integration Tests Passed (Run 28585060010)Branch: Test Results
Application Logs (last 20 lines)receiver-echo-server.log sender-echo-server.log sender-test-client.log receiver-test-client-iperf.log sender-test-client-iperf.log Full Application Logs (last 200 lines each)receiver-echo-server.logsender-echo-server.logsender-test-client.logreceiver-test-client-iperf.logsender-test-client-iperf.log
|
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.
The engine bug the TCP smoke tiers (#111) isolated on real hardware: a single TCP round-trip works, but sustained exchange on one connection hangs (Graviton
tier1-tcp-echo:bidir_echo✅,bidir_multi❌ hung to timeout).Root cause
handle_established's ACK path prunes the retransmit queue but never drains the acknowledged bytes from the front ofsend_buf, and never clearshas_unacked_data. After the first send+ACK,send_bufstill holds the stale acked bytes whilealready_sent_offset(derived from the now-empty retransmit queue) resets to 0. The next small write then makessend_buf.len()exceed one MSS withhas_unacked_data == true, so Nagle withholds it forever → the peer never gets iteration ≥1 → both sides idle → client blocks (the ~60s harness kill). The receive window is fully open — it's not flow control.Fix
On cumulative ACK: drain the acked bytes off the front of
send_buf, rebase surviving retransmit-entry offsets, and clearhas_unacked_datawhen nothing is in flight (snd_una == snd_nxt).Verification (offline, no EC2)
New
tests/loopback_stall_repro.rswires a clientTcpEngine↔ serverTcpEnginethrough in-memory frame queues and drives 20 sequential 64B echoes on one connection — mirroringtcp-test-client --mode bidir --count 20. Fails before (stalls at exchange 2), passes after. All 221 unit + property/integration tests green.Found via a focused diagnostic that reproduced the EC2 symptom in a <1ms unit test. Follow-up: the same
send_bufdrain is missing inhandle_fin_wait_1/handle_close_wait(teardownsend_bufleak, not a stall).🤖 Generated with Claude Code