fix(windows): preserve data-path backpressure - #3704
mozharovsky wants to merge 1 commit into
Conversation
92c33bd to
b320d90
Compare
|
Independent verification from the Hummingbird side, on a clean windows-2022 GitHub runner (Swift 6.3.3). Applied this branch (
Both were failing with "stream ended at an unexpected time" before this change. With it, the full streamed-body set is green ( |
Mirror Posix.syscall for Winsock data calls and cover both wrapper variants plus full stream delivery under forced backpressure.
b320d90 to
15c10a3
Compare
|
Rebased onto current main after #3699 landed, the conflict was the expected overlap in BSDSocketAPIWindows.swift. The two wrappers, the five routed data-path sites and the tests are functionally unchanged from b320d90 that @Budoman verified. #3700 is still open, I'll rebase again if needed once it lands. |
|
Re-verified the rebased branch ( Targeted the streaming family that was previously red: Pin for reproducibility: |
|
Thanks for re-running it on the rebase @Budoman! @jakepetroules this should be ready for a look whenever you get a chance - rebased over #3699 and Hummingbird has verified both the original branch and the rebase independently on a clean runner. |
|
This looks reasonable to me. @fabianfett / @glbrntt can we get a maintainer review please? |
Treat
WSAEWOULDBLOCKas nonblocking backpressure across the existing Windows data path.Motivation:
Nonblocking Winsock operations report backpressure by returning
SOCKET_ERRORand settingWSAEWOULDBLOCK. The existingacceptandconnectwrappers already classify this condition as nonfatal, and #3699 does the same for its newsendmmsgimplementation. The existingrecv,send,writev,recvmsg, andsendmsgwrappers instead throwIOErrorbefore the channel can wait for readiness.In a downstream production workload, this surfaced as Windows error 10035 and an
uncleanShutdownabout two seconds into bulk TLS uploads, while low-volume traffic remained unaffected. The same mapping has been shipping in a downstream fork and eliminates the failure.Fixes #3697.
Fixes #3702.
Modifications:
winsockSyscall(where:_:)overloads that mirrorPosix.syscalland use the samewhere function: String = #functionconvention.WSAGetLastError()immediately when the call returnsSOCKET_ERROR.DWORDout-parameter. It reads the count only after success and ignores any value a failed call leaves in that slot.recvandsendthrough the single-closure overload, and routedrecvmsg,sendmsg, andwritevthrough the transferred-count overload without changing any public API.acceptandconnectunchanged. Unifying their nonblocking handling is a separate NFC follow-up.Neither overload has an
EINTRretry loop.WSAEINTRonly arises from the legacyWSACancelBlockingCall, which does not apply to the nonblocking sockets used by these data paths.Using
#functiondeliberately changes the fiveIOErrorreason strings from bare implementation names such as"recv"and"WSASend"to the full caller signatures such as"recv(socket:buffer:length:)"and"writev(socket:iovecs:)". The error codes and public API are unchanged.This complements #3699 and #3700. I am happy to rebase over them when they land and fold the
sendmmsgcall into the shared wrapper as a follow-up.Result:
The stream regression test fails on unpatched Windows with
WSAEWOULDBLOCKsurfaced asA non-blocking socket operation could not be completed immediately.Both wrapper overloads and the unchanged stream regression pass with this change on Windows x64 using the current development toolchain:The full macOS suite passes with 2,460 XCTest tests and 276 Swift Testing tests. The Swift 6.2 formatter check, API breakage check, unacceptable-language check, license-header check, and broken-symlink check all pass locally.