Repository navigation
Sockets: specify and pin the close() cancel contract on WinSocket/POSIX - #1830
Merged
Merged
Conversation
Specifies what close()/close_connection() do to in-flight operations: - close()/close_connection() cancel any in-flight send(): it returns promptly with the number of bytes actually written so far (best effort). - No further on_receive_data callbacks are started (one already running is allowed to finish); the receive side stops immediately. This lands on the abstract client/server classes (the contract) and on the WinSocket and POSIX backends (how each meets it). The WinSocket and POSIX implementations already satisfy the contract - the send loops re-check the state every iteration and the close paths notify/shutdown the socket, so an in-flight send ends within a millisecond - so this PR changes no send/close behavior; it states the guarantee and proves it with two new cross-backend tests: - "close() cancels an in-flight send": a peer that never reads keeps a 256 MB send in flight; close() must end it. The send and close must both return within 60 s (without cancellation this is an infinite hang), and the send may not report more bytes than the payload. - "close_connection() cancels an in-flight send": the same from the server side, plus the server must be back to LISTENING. The Qt backends get the same guarantee in follow-up changes; the tests pass on them today (their userspace buffering completes the send before the close) and will pin the Qt behavior once it lands.
This was referenced Nov 2, 2026
Both new tests allocate a 256 MiB payload; on the Qt backend the in-flight send data holds a second 256 MiB copy of it. Set m_memory to the 512 MiB worst case (overreported on the backends that don't copy) so the parallel test runner can schedule them within its memory budget.
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.
Task #2 from the close()-cancel plan: specify the contract, make WinSocket and POSIX explicit about meeting it, and pin it with cross-backend tests.
The contract (now on the abstract classes)
close()/close_connection()cancel any in-flightsend(): it returns promptly with the number of bytes actually written so far (best effort).on_receive_datacallbacks are started; a callback already running is allowed to finish (the receive side stops immediately).send()docs now list "close() is called" as a way the wait ends, and the return value covers the cancelled case.Compliance
The WinSocket and POSIX backends already satisfy the contract — verified by reading all four: every send loop re-checks the state under the lock at each iteration,
close()stores DESTRUCTING + notifies the send's CV wait (client) /close_connection()+close()flip the state to LISTENING/DESTRUCTING andshutdown()the socket, and the error paths don't record spurious errors when the close is in progress. So no send/close behavior changes in this PR; the backend headers now state the mechanism next to eachclose()/close_connection(). (The one Win behavior that didn't match the other backends — returning on the first partial::send()— is #1829.)Tests (new, run on every backend)
close() cancels an in-flight send— the server never reads, so a 256 MBsend()stays in flight; after a 100 ms head start the test callsclose()and asserts both the send and the close return within 60 s (without cancellation this is an infinite hang, so the threshold only proves boundedness and tolerates the starved-CI cases the other tests skip on) and that the send never reports more bytes than the payload.close_connection() cancels an in-flight send— same from the server side, plus the server must be back to LISTENING (polled within the shared budget, per the pending-close pattern).On the Qt backends these tests currently pass vacuously (Qt's userspace buffering completes the send before the close); the real Qt cancellation lands in the follow-up Qt PRs, and the tests will pin it then.
Ordering
File-disjoint from #1829, but please merge that one first — its Win client alignment is what the "loops until everything is written, the deadline elapses, or close() cancels the send" doc wording describes end-to-end.