Repository navigation
feat(libkernel): apply SO_RCVTIMEO and SO_SNDTIMEO on a guest socket - #5617
Conversation
|
Evidence for head Which catalog titles ask for the two options
Method: as in #5565. The 65 release archives listed in the catalog were downloaded (SHA-256 equal to the catalog records) and the 63 TestsWindows: guest_socket_timeouts, 20 repeated runsWindows: the ten socket tests (guest_udp times out on this machine on main too)Windows control: Udp.cpp of the first commit, new testLinux: the eleven socket testsLinux control: Udp.cpp of the first commit, new testAI-assisted (Claude Code). |
adriadam10
left a comment
There was a problem hiding this comment.
This is stacked on #5565, which still needs to be ported to main: #4796 is merged, so setsockopt/getsockopt now delegate to GuestSockets::SetSocketLevelOption/GetSocketLevelOption in Socket/src/SocketOptions.cpp, with per-socket state in Socket::options under modeMutex. The branch conflicts with main in Udp.cpp for that reason.
Udp.cppOwnOption/SetOwnOption/GetOwnOptionis a second dispatcher in front of that code. Please putSO_SNDTIMEO/SO_RCVTIMEOintoSocketOptions.cppand keep the timeouts inSocketOptionStaterather than two separate atomics onSocket.- Rebase after #5565 is ported so this PR carries only its own commit. Keep the
WaitForTransfercalls insend/recv/sendto/recvfrom/recvmsg(andread/write/sendmsgas the description says; I only see the five in the diff, please check the others go through them). - The description names RommPS and SwanStationPS5 from their sources only. If one of them stops on this on current main, add the title ID and the log line, as requested for the other socket PRs.
Happy to re-review once it is rebased on the ported base.
8b4309f to
514d003
Compare
|
@adriadam10 done. The branch is now the ported #5565 (
Results at
AI-assisted (Claude Code). |
…est socket Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
514d003 to
17ded56
Compare
|
Rebased both branches onto
Results: Linux, all 13 socket tests pass on this head and all 12 on #5565's. Windows, 12 of 13 and 11 of 12 ( AI-assisted (Claude Code). |
|
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.
|
adriadam10
left a comment
There was a problem hiding this comment.
Re-checked against the points of the previous review: the timeouts now live in SocketOptions.cpp with the values in SocketOptionState under modeMutex, the branch is one commit on current main, and read/readv go through recv, write/writev through send, sendmsg through sendto. Built and ran the libs/libkernel ctest scope (174 tests, all pass), guest_socket_timeouts included, and it fails with the non-test changes reverted. Merging once the Windows build is green.
|
Cross-validation on Windows 11 Pro 26220, NVIDIA RTX 4070 (596.21), MinGW GCC 15.2, PR head #5565 is already on RommPS (PPSA76677)
SwanStationPS5 (PPSA98510)
Other titles
Tests
Suite. Conventions. Review. The checks and conversions match the pinned FreeBSD 11.4 sources: One difference on Windows. On a TCP socket that was never connected, FreeBSD's Written with Claude Code assistance. |
What
The first commit of this branch is #5565 as ported to
main(7e79f36ed), unchanged. This PR is the second commit. It adds two options to the same structure: the handling is inSocket/src/SocketOptions.cpp, and the values are inSocketOptionStateundermodeMutex.setsockoptandgetsockopton a socket fromsocket()failSO_SNDTIMEO(0x1005) andSO_RCVTIMEO(0x1006) withENOPROTOOPT, and a blocking receive never times out. Now:sosetoptdoes. It is a 16-bytetimeval; a negative field or 1000000 microseconds or more fails withEDOM, and more thanINT32_MAXseconds becomes the longest wait. It is stored inSocketOptionState::sendTimeoutandreceiveTimeoutand read back through the same 32.32 fixed-point conversion (tvtosbt,sbttotv), so half a second reads back as 499999 microseconds and one microsecond as 0. A short or null value fails withEINVAL, as for the other options there.send,recv,sendto,recvfromandrecvmsgon a blocking socket wait that long for the host to report the socket readable or writable, then fail withEWOULDBLOCK. The wait reads the timeout and the non-blocking flag together undermodeMutex. A call withMSG_DONTWAIT, whichmainnow accepts on Linux, does not wait, as FreeBSD'ssoreceiveandsosenddo not sleep for it. The other calls reach these five:readandreadvgo throughrecv,writeandwritevthroughsend, andsendmsgthroughsendto. A non-blocking socket and a timeout of 0 behave as before.sonewconncopies them, in the same locked block that copies the other options.SocketOptions.cppentry ofdocs/dev/debt/libkernel.md:sendfileignores the send timeout.Titles that ask for them:
main90d0cb481, not part of this PR, showssetsockopt(SOL_SOCKET, SO_RCVTIMEO)andSO_SNDTIMEOwith 2 s failing withENOPROTOOPT(42). It happens three times while the title checks for a local FTP server. The title does not stop there: each check ends after about 2 s withno answer.mainboth calls fail, and the sockets wait without a limit. This is read from their sources; neither title was run.SO_RCVTIMEOis set by 27 titles andSO_SNDTIMEOby 24, and each is read by 19. The option was recovered at 840 of 2661setsockoptcall sites, so these are lower bounds. The lists and the method are in this comment.No title is known to stop on these options on
main.Related: #5565 is the first commit of this branch and the base of this change. Once it lands, this PR is rebased to carry only its own commit.
Tested
Windows 11, Ryzen 7 5800H, WinLibs MinGW-w64 GCC 15.2.0 r7; Ubuntu 26.04 under WSL, GCC 15.2.0. Release,
BUILD_TESTING=ON, root configuration withFFMPEG_PREBUILT_DIRpointed at an already unpacked package. Head17ded56f1, on #55657e79f36ed, onmain4c1de0c0c.guest_socket_timeoutscovers values and read-back with FreeBSD's rounding,EDOM, short arguments and a truncated read. It checksEWOULDBLOCKfromrecv,recvfrom,recvmsgandreadon a datagram socket and fromrecvon a stream socket. On Linux it checks thatrecv,recvfromandrecvmsgwithMSG_DONTWAITfail withEWOULDBLOCKat once under a 60 s timeout; without the check for the flag the test runs into its 20 s limit. It also covers a datagram received while the timeout is set, inheritance by an accepted socket, and a streamsendthat stops withEWOULDBLOCKonce the peer's buffers are full. Its only clock check is a lower bound: a 60 ms wait does not end before 59 ms.SocketOptions.hpp,SocketOptions.cppandUdp.cppof the first commit and the new test,guest_socket_timeoutsfails on both systems withsocket timeout check failed at line 93, errno 42.guest_udptimes out after 20 s, and does so onmaintoo: a VPN on this machine drops IPv6 loopback traffic, so a datagram to[::1]never arrives, even between two plain host sockets.guest_udpincluded.git diff --checkpass.Not tested: any title, a console, macOS, the full test suite. Full PR builds need a collaborator's manual Build run.
Checklist
mainthrough feat(libkernel): answer TCP_NODELAY, SO_NOSIGPIPE and SO_NBIO on a guest socket #5565, the first commit of this branch; no other open PR implements these two options.mdfiles or images added to the repository🤖 Generated with Claude Code