[Windows] Report would-block from the socket read and write paths - #3708
Open
jakepetroules wants to merge 2 commits into
Open
[Windows] Report would-block from the socket read and write paths#3708jakepetroules wants to merge 2 commits into
jakepetroules wants to merge 2 commits into
Conversation
`CNIOWindows_sendmmsg` was a stub that asserted and then called `abort()`, so any code path reaching it took the whole process down. That is the datagram write path whenever more than one write is pending, which made `DatagramChannelTests` and `RawSocketBootstrapTests` unrunnable: both aborted partway through, taking the rest of the test process with them. Winsock has no `sendmmsg`, so emulate it by sending the messages one at a time, which is what the Darwin shim does for the same reason. Implement it in Swift alongside the existing `sendmsg`, rather than filling in the C shim, since everything needed is callable directly from Swift; the now-unused C stub and its declaration are removed. The `WSASendMsg` extension function is resolved once per batch rather than once per message, so the lookup is hoisted into a helper shared with `sendmsg`. The error semantics match the other implementations: a failure on the first message is reported to the caller, with `WSAEWOULDBLOCK` reported as `.wouldBlock` the way the POSIX implementation's `syscall(blocking: true)` wrapper does, while a failure after some messages have been sent is reported as a short send for the caller to retry. This does not yet make either test suite pass, so no skips are removed here. Both now get further before failing for unrelated reasons: the datagram tests reach a `fatalError` in `PendingDatagramWritesManager`, which reads `errnoCode` on an `IOError` that carries a winsock-domain code, and the raw socket tests fail with "Already closed". Crucially, neither aborts the test process any more.
`recv`, `send`, `writev`, `recvmsg` and `sendmsg` turned every `SOCKET_ERROR` into a thrown `IOError`, including `WSAEWOULDBLOCK`. NIO's sockets are always non-blocking, so the one error that is not an error at all -- the send buffer being full, or no datagram being available yet -- arrived as a failure. NIO then treated ordinary backpressure as a fatal I/O error and closed the channel. Return `.wouldBlock(0)` for `WSAEWOULDBLOCK` instead, which is what the POSIX implementations produce through their `syscall(blocking: true)` wrapper. This fixes `ChannelTests.testWritevLotsOfData`, which wrote enough data to fill the send buffer and then failed with "A non-blocking socket operation could not be completed immediately".
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.
Motivation:
Depends on #3699, which is the first commit on this branch. It changes
sendmsg, which this PR then touches. Please review only the second commit, or wait until #3699 is merged.recv,send,writev,recvmsgandsendmsgthrew anIOErrorfor everySOCKET_ERROR, includingWSAEWOULDBLOCK.NIO always uses non-blocking sockets. So a full send buffer, or no datagram yet, arrived as a failure. NIO then treated normal backpressure as a fatal I/O error and closed the channel.
Modifications:
Return
.wouldBlock(0)forWSAEWOULDBLOCK. This is what the POSIX code produces through itssyscall(blocking: true)wrapper.Result:
A write that fills the send buffer now creates backpressure, instead of closing the channel.
This fixes
ChannelTests.testWritevLotsOfData, which failed with "A non-blocking socket operation could not be completed immediately". That test only runs whenChannelTestsis enabled on Windows, which another PR does.Tested on a Windows ARM64 machine (Swift 6.3.2): builds, and part of a larger branch where a full
swift testpasses (2360 tests, 0 failures).