Repository navigation
Conversation
A peer that half-closes with data still queued, as Linux does on shutdown(SHUT_WR), puts its FIN on the last data segment. In Established such a segment was handled by its flags alone: - ACK|FIN in sequence took the FIN without its payload, so the last bytes of the stream never reached the reader and the ACK number fell short of them. - ACK|PSH|FIN matched no arm and was dropped without an ACK, every retransmission of it too. - A FIN segment arriving ahead of a gap was dropped without an ACK. RFC 9293 §3.10.7.4 processes the segment text before the FIN bit. The payload is now received like that of any other data segment, PSH or not, and the FIN is taken only once every byte before it has arrived. A FIN that is not yet in sequence leaves its payload queued and is taken when the peer retransmits it. Refs narrowlink#111
A FIN closes only the peer's direction (RFC 9293 §3.6). On a FIN in Established with nothing in flight, the stack answered at once with its own FIN and moved to LastAck, and in CloseWait any ACK with nothing in flight did the same. A client that sends a request and then calls shutdown(SHUT_WR) therefore had the reply cut off as soon as the data already written was acknowledged. A reader in CloseWait also got Pending rather than end of stream, and waited for a timer. After the peer's FIN the stack now: - acknowledges it, wakes the reader, which gets every byte sent before the FIN and then end of stream, and keeps the write side open; - in CloseWait, acknowledges a retransmitted FIN, or segment carrying it, and answers duplicate ACKs with a fast retransmission, as in Established; - sends its own FIN when the application shuts down, once everything written is acknowledged, or when it drops the stream. The FIN sent on shutdown is retransmitted by the LastAck timer, which now lives at module level so poll_shutdown can start it. A write after it fails with BrokenPipe. close_wait_timeout remains the backstop for an application that never shuts down, and it is now armed on every FIN rather than only when data was in flight. It counts time without progress, so a reply that is still being written and acknowledged is not cut after a fixed delay. A writer still waiting for window when the connection ends is woken. Refs narrowlink#111, related to narrowlink#89.
…ation Preserve frontierlabs-admin commits from PR narrowlink#112 and SajjadPourali shutdown wakeup fix from PR narrowlink#114. Resolve their interactions and cover FIN retransmission, crossed closes, and bounded detached cleanup with unit tests and a Linux TUN matrix.
Collaborator
|
Thanks for putting this together. I haven’t started reviewing the code yet. I review changes by hand, and this PR is too large for me to review thoroughly in one pass. Could you split it into smaller PRs, starting with the half-close fix and its regression tests? |
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.
Client-initiated half-close must deliver the bytes carried by FIN and leave the application free to send its reply. This PR integrates frontierlabs-admin's work in #112 and preserves SajjadPourali's deferred-shutdown wakeup fix from #114, following Musyg's report in #89. Both original commits from #112 remain in the branch history.
The combined change wakes shutdown independently in Established and CloseWait, validates ACKs before releasing outstanding data or acknowledging FIN, handles crossed FINs, and retransmits FIN at its original sequence, including wraparound. Writes stop after the local FIN; unacknowledged data survives CloseWait timeout handling. Dropping a stream inside Tokio continues the close handshake with bounded cleanup. FIN retries use the current RTO as a floor and exponential backoff; inactive FinWait2 expires, and a repeated peer FIN restarts TimeWait.
The expanded Linux tests found an additional defect: after the final FIN ACK, a delayed reader notification could emit a window-update ACK. Linux had already closed and answered with RST even though application data delivery had succeeded. Window-update notifications now run only while the receive direction remains open. A deterministic regression fails on the previous revision and passes with this fix.
Validation on Phoenix5-Orion, Ubuntu 24.04.5, Linux 7.0.0-42-generic and Rust 1.95.0, against base
44c63441d15888c9275844af4364466c0d4ed288:-D warningspassed.These results were collected on Phoenix5. The exact dependency lockfile, failing pre-fix regression, intermediate diagnostics, final logs, matrix auditor and SHA-256 manifest are retained in the qualification bundle. The published diff against the pinned base is byte-identical to the verified patch, SHA-256
e9fbad35c8eb1aa6242627c6d4c4f27cb62b9fa1bb828840dbd422aa7856971e.After building and generating Cargo.lock, run:
Linux, TUN, iproute2, Python 3, util-linux and permission for
sudo -n unshare --netare required. Qualification details and reset classification describe the assertions and reproduction commands.The lifetime after drop, use of
config.timeout, and FIN retry limits remain behavior choices for maintainer review; their implemented boundaries are tested. This work does not add an adaptive RTT estimator or claim exhaustive TCP conformance. No security exploit has been demonstrated.