Repository navigation
fix(net): complete zero-length TCP receives without waiting on Windows - #8686
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Independent Windows run of this PR, in case it helps review. Environment: Windows 11 10.0.26300 (same build as the PR description), Ryzen 7 9800X3D, WinLibs GCC 15.2.0 r7 UCRT, Release, BUILD_TESTING=ON. I built only the Results:
Not run: the standalone Winsock urgent-data probe, Linux/macOS, the full repository CTest, and any title or console run. These are synthetic loopback tests only. AI-assisted: yes |
|
The independent Windows run materially strengthens the report: it reproduces the 24/24 head and 17/24 baseline split on the same OS build, using only the production-file swap for the baseline. It also shows |
54d4adf to
5810884
Compare
|
@Ondra97 I'm adding SO_PROTOCOL readback in #9930. Both branches add an assignment to the accepted socket next to Would landing #9930 before this PR work for you? Mine already follows #9543/#9525. If yours needs to go first, I can rebase and keep both assignments instead. I've left mine as a draft until we settle the order. |
|
@Queuse I confirm that #9930 landing before #8686 works for my plans. I added #9930 to #8686's Depends on line for this merge order; your existing #9525 -> #9543 -> #9930 sequence stays intact. I checked both patches. When #8686 is rebased after #9930 lands, the accepted socket must retain both This is merge-order coordination only. No source changes or combined runtime test are claimed by this reply. |
|
Thanks, @Ondra97. I've updated #9930 with the agreed order: #9525 → #9543 → #9930 → #8686. Both accepted-socket assignments, test registrations and debt entries need to stay when yours rebases. The order with #8686 is settled; I'm keeping #9930 as a draft while the separate #6044 test-reorganization order is still pending. |
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 522e32e and FreeBSD 11.4 sys/kern/uipc_socket.c (soreceive_generic, lines 1568-1583):
- The ordering for the established, error-free case matches FreeBSD 11.4: the ENOTCONN check for a never-connected stream, then the
uio_resid == 0return, then the nonblocking EWOULDBLOCK check. The remaining states are listed in the debt file, which seems right to me. - Possible macOS build problem after rebase: on main,
wait_for_local_addressis now compiled under#if defined(_WIN32) || defined(__APPLE__)(Export.cpp:690-759, widened in 78983da after your base). This PR's new stream branch usesWSAPOLLFD/WSAPollwithout a_WIN32guard, while every otherWSAPolluse in the file has one (e.g.wait_for_datagramaround :723-732), and nothing in the tree definesWSAPOLLFDfor other hosts. The branch still merges textually (3-waygit merge-treeshows no conflict markers), so this would only show up when building on macOS; I couldn't build there. Would you consider guarding the new stream branch with#ifdef _WIN32, so macOS keeps main's current early return for streams? - Main doesn't already contain this change: its
wait_for_local_addressstill returns 0 immediately for every stream socket (:737).
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
@harisibrar1 Fixed in The stream branch now guards its WinSock logic with All production libraries, the relinker and selected fixtures built on the final commit. The focused Windows selection passed 62/62, including all 24 empty-stream cases, with zero skips; 368 patched libraries loaded. Conventions, strict main-sync and whitespace checks passed. I have not run a native macOS build or runtime test. The agreed #9930 merge order remains in Depends on. AI-assisted: yes. |
|
Re-checked against current main: the new |
What
On Windows, a zero-length
sceNetRecv,sceNetRecvfromorsceNetRecvmsgon a healthy connected TCP socket can wait for peer data, or returnEAGAINwithMSG_DONTWAIT/SO_NBIO. A caller asking to receive no bytes should complete with zero without waiting or consuming queued stream data. Fresh unconnected sockets also expose Winsock'sEINVALthroughrecvfrom/recvmsg, instead ofENOTCONN.The Windows receive-preparation helper previously bypassed every stream socket. The exports then reached the readiness check or native receive even when their requested size was zero. I extend that existing helper to complete an empty receive only when
getpeernameidentifies a connection and a nonblockingWSAPollcheck reports no error, hangup or invalid descriptor. It does not read or clearSO_ERROR. Existing export paths set the empty sender length and clearrecvmsgoutput flags/control length while leaving payload, address and control storage untouched.The stream-specific branch is guarded by
_WIN32. The helper is also compiled on macOS in current main; macOS keeps its existing immediate stream return and never seesWSAPOLLFDorWSAPollfrom this branch.For a stream that has never attempted a connection, the helper reports
ENOTCONN, including bound and listening sockets. Connection attempts and accepted connections are tracked so an unsuccessfulgetpeernamedoes not invent that fresh-socket result for a pending or failed connection. Pending-error, disconnected and connecting paths continue through the existing host receive path; their exact console error/output precedence remains documented as debt. Nonempty receives and UDP keep their existing paths: in particular, a zero-length UDP receive still consumes a queued datagram.The behavioral reference is FreeBSD 11.4
soreceive_generic, lines 1568-1583: an unconnected connection-required socket fails first, and zero residual length returns before the nonblocking wait decision. This is a BSD reference, not a new PS5 measurement.I add 24 Windows runtime regressions: all three exports with an empty stream,
MSG_DONTWAIT,MSG_PEEK | MSG_DONTWAIT,SO_NBIO, queued data, null/empty vectors, invalid descriptors/flags and fresh/bound/listening sockets, plus a UDP consumption control for each export. The tests check output canaries and bound the empty receive to 500 ms, with cleanup if the baseline waits.Tested
Latest validation (2026-10-24)
Base
522e32e3b618154e0e221a9f4d28da87dd2cf187; submitted and tested head3d0c15528847dc042ad50428c7c21c37bc7ad7f1. Windows 11 Pro build 26300, Ryzen 9 7900X, 31.66 GiB RAM, RTX 4070 Ti SUPER (no GPU tests); WinLibs GCC 15.2.0 UCRT POSIX SEH r7, Ninja Release,BUILD_TESTING=ON, bundled dependencies and the existing FFmpeg prebuilt tree.patched_librariesloaded 368 libraries, zero failed. The original regression source is unchanged.58108841b1d351689df4f32091bbdb209ce6895dand the pinned base exposesWSAPOLLFD/WSAPollwith_WIN32undefined and__APPLE__defined. The final source excludes both and retains the streamreturn 0. Windows preprocessing is byte-identical before and after the guard. This checks the conditional source branch, not a native macOS build.No native macOS build/runtime, title or console run, default all-target build or complete CTest suite was performed. The earlier Windows red/green results and host-specific observations are retained below with their original revision IDs.
Commands, from the source tree with the documented toolchain on PATH,
FFMPEG_PREBUILT_DIRset and the sibling Release build configured:Historical validation on earlier revisions
Environment: Windows 11 Pro 10.0.26300; Ryzen 9 7900X, 32 GB RAM, RTX 4070 Ti SUPER (no GPU tests); WinLibs GCC 15.2.0 UCRT POSIX SEH r7; Ninja Release with
BUILD_TESTING=ON, Python 3.14 and the existing FFmpeg prebuilt dependency. These are synthetic loopback tests calling the implemented exports. No original PRX/SPRX modules, console or PRAGMATA run was available. This is a partial host-side correction related to #8005; title-level impact and completion of the overall audit remain unverified.Historical October 22 validation: base
a6dcea9279b53b3a39c13695eac66f7c8b5b503c, head58108841b1d351689df4f32091bbdb209ce6895d. The original production patch and all 24 new test cases are preserved; the documentation resolution retains the current-main debt entries and adds only this PR's receive-state caveat.With the unchanged production file from current main and the final regression fixture, 7/24 tests pass and 17/24 fail. After restoring the fixed production file and rebuilding, all 24 pass. The queued-data and UDP consumption controls pass in both runs.
The production build and selected fixtures pass without a source overlay. The expanded runtime selection passes 62/62 with zero skips in 10.14 s, covering all 24 empty-receive cases, datagram and guest-buffer controls, refused connections, nonblocking/blocking transfers, TCP, SSL and patched-library loading.
Conventions, strict main-sync and whitespace checks pass. The overlap tool identifies the existing remote version of this PR and requests manual review of shared files in #5836, #6044, #6879, #7201, #8204, #8225, #8232, #8234, #8235, #8254, #8362 and #9076. Their changes cover other transports/options, test paths, platform handling, address parsing, diagnostic/configuration exports, socket information and allocation flags; none supplies this empty-stream receive behavior. I also reviewed related connection, timeout and closed-stream-readiness changes; this is source comparison, not runtime validation of combined open PRs.
The default all-target build and complete CTest suite were not run on the rebased head. The original build, probe and host-specific
guest_pollobservations below are historical and pinned to their stated revisions.Revision under test:
54d4adf7f58fe0d9b454b4d377a66e5c454020f9, based onaa1b1bfcea0e9fc2a457577ad8dce94010da7cad. The before/after comparison uses the same final test source, build configuration and dependencies, replacing only the production file changed by this PR with main'sExport.cpp(blob6a679a251fdd22b452127a24db2a0c50b838e268, also unchanged ata6a4101e190ec408c3ed471d20f06872cae1b4a3). Each table entry is one CTest invocation, not a performance benchmark.sceNetRecvcasessceNetRecvfromcasessceNetRecvmsgcasesThe failing base cases cover waiting/
EAGAINon empty streams andEINVALon freshrecvfrom/recvmsg. The queued-data and UDP controls passed before and after.Validation of the revision above:
patched_libraries(354 loaded, 0 failed).check_conventions.py,git diff --checkand the test-registration audit passed; the latter reported 1,127 test executables and no built-but-unregistered executable.guest_polltest failed with exit0xc0000409. It linkslibkernel/libc, notlibSceNet; neither its source nor its kernel socket implementation is changed here. A standalone native Winsock urgent-data probe on this host also reported no exceptional readiness and delivered the urgent byte through normal receive even withSO_OOBINLINE=0. This remains an unresolved host/test limitation; I do not claim a full repository CTest pass or a fix for it.The full Windows test build requires the single-file test correction from #8310 at
ca55e46f98ad85861dd7567abad0c028274daca1. Without it, this MinGW toolchain rejects the existingalignas(65536)static array inImageArrayWindowPastLayerLimit.cppbecause its supported maximum is 8192. I applied only that reviewed test-file change for validation and restored the original bytes afterwards; it is absent from this PR. The build result therefore applies to this head plus that test prerequisite, not an unqualified standalone-head build.Reproduction after configuring the documented Windows toolchain and dependencies: put the GCC toolchain and Python on
PATH, and setFFMPEG_PREBUILT_DIRin the environment to the existing FFmpeg prebuilt tree. Fetch #8310's exact revision before generating its test-only patch.For a failing baseline, retain this PR's test source and CMake registration, replace
core/libs/prx/libSceNet/Export.cppwith the file from the base revision, rebuild the target and repeat the same CTest command. Restore the fixed file afterwards. This isolates the sole production-file change rather than attributing unrelated changes on moving main to the fix.Overlap review: the scan found no conflicting changed lines with #8653 (per-call nonblocking transport) or #8598 (epoll registration lifetime). I also compared their shared implementations: neither handles empty stream receives, and their transport/state changes compose with this guard; static three-way merges of all three shared files were clean. The kernel-only empty-receive change in #8492 is separate from these
libSceNetexports. I manually reviewed the files the checker could not compare in #5836, #6044, #6879, #7201 and #8204: peer transports/options, test organization, host includes/macOS resolver handling and Ethernet address parsing do not implement this behavior. This is source review, not runtime validation of combined PRs.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)