Repository navigation
fix(libSceNet): register libkernel sockets with sceNetEpollControl - #11242
Conversation
Human Fall Flat (PPSA02429) opens its sockets with libkernel's socket() and registers them with sceNetEpollControl. libSceNet only looked in its own socket table, so every ADD and DEL failed with SCE_NET_ERROR_EBADF. The game retries without pause: on box 2 it made more than 1.6 million failing calls on 14 descriptors in its first seconds, all of them libkernel sockets. sceNetEpollControl now accepts a descriptor libkernel has open as a socket, as sceNetSocketClose and sceNetSetsockopt already do for MXGP 26 (PPSA30839). sceNetEpollWait looks up the host socket of such a registration through libkernel after g_mutex is released, because that lookup asks libSceNet's own table for descriptors libkernel does not own, and keeps the owner alive across the host poll. libkernel's close() retires the registrations of the closed socket; a wait drops a registration whose socket is gone. guest_sce_net registers a libkernel UDP socket, waits for it to become readable and then writable, deletes it twice, closes registered sockets through close() and sceKernelClose and waits again, and checks that a descriptor that was never open still fails with EBADF.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I found one correctness issue in close handling for the new kernel-socket registrations: sceKernelClose bypasses the cleanup callback, so a reused descriptor can inherit the old epoll registration before the next wait prunes it. I requested cleanup on that path and a reuse regression. Current-main conventions and diff-check pass; hosted product jobs were skipped by path policy. The author reports Windows tests, which I did not rerun locally.
| std::lock_guard lk(g_mutex); | ||
| if (g_socks.count(descriptor) == 0) return false; | ||
| if (g_socks.count(descriptor) == 0) { | ||
| for (auto& epoll : g_epoll_socks) epoll.second.erase(descriptor); |
There was a problem hiding this comment.
This cleanup only runs when KernelCloseSocket_nid_no_patch is called. sceKernelClose in libkernel/File/src/Open.cpp closes GuestSockets directly and does not invoke that callback; the registration therefore remains until a later sceNetEpollWait fails to look up the descriptor. If the descriptor number is reused for another kernel socket before that wait, KernelLookupSocketNative succeeds for the replacement and the stale epoll registration watches the new socket. Please retire these registrations on the sceKernelClose path too (or key them by stable socket identity/generation), and add a regression that closes a registered socket with sceKernelClose, immediately reuses its descriptor, and proves the old registration cannot report readiness for the replacement.
|
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.
|
|
@MarcZX10 thanks, you're right. Fixed in 3cbd5a4: The regression you asked for is in |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 3cbd5a4be3366ab9798c495f94809e03dc11dd05. The new sceKernelClose path calls the registered libSceNet closer; for a libkernel-owned socket, CloseSocketForKernel retires its epoll entry under g_mutex, then the existing GuestSockets::Close path closes the descriptor. The regression registers a libkernel socket, closes it with sceKernelClose, reuses the descriptor, verifies the new socket is not reported through the old registration, then registers it and confirms readiness/data. This addresses my prior stale-registration finding. Exact-head git diff --check upstream/main...HEAD and tools/check_conventions.py --base upstream/main pass. Hosted conventions, labels, eligibility and report pass; product/relinker jobs skip by path policy. The focused Windows guest_sce_net run is author-reported and I did not rerun a local product build. No remaining blocker found.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The sequential close/reuse regression covers cleanup after a completed close, but sceNetEpollControl still has a close/reuse race for libkernel-owned sockets: GuestSocketIsOpen_nid_no_patch(id) checks GuestSockets::sockets under socketsMutex, releases that lock, and only afterwards does this function reacquire g_mutex and insert the numeric descriptor into g_epoll_socks. If another thread closes id and opens a replacement in that gap, the close callback can erase no registration, then this call can register the replacement descriptor without validating its identity. A subsequent wait can therefore deliver readiness for the replacement under the old registration. Please make validation and registration retain/verify the socket lifetime (or otherwise serialize close/reuse with this insertion), and add a deterministic regression for close/reuse racing ADD. The existing sequential reuse test does not exercise this ordering. I did not run a product build locally; the focused Windows test is author-reported. Exact-head diff check and current-main conventions pass; hosted product jobs were skipped by path policy.
…o (upstream boykopovar#11242, boykopovar#11243, boykopovar#9948) (#25) * fix(libSceNet): register libkernel sockets with sceNetEpollControl Human Fall Flat (PPSA02429) opens its sockets with libkernel's socket() and registers them with sceNetEpollControl. libSceNet only looked in its own socket table, so every ADD and DEL failed with SCE_NET_ERROR_EBADF. The game retries without pause: on box 2 it made more than 1.6 million failing calls on 14 descriptors in its first seconds, all of them libkernel sockets. sceNetEpollControl now accepts a descriptor libkernel has open as a socket, as sceNetSocketClose and sceNetSetsockopt already do for MXGP 26 (PPSA30839). sceNetEpollWait looks up the host socket of such a registration through libkernel after g_mutex is released, because that lookup asks libSceNet's own table for descriptors libkernel does not own, and keeps the owner alive across the host poll. libkernel's close() retires the registrations of the closed socket; a wait drops a registration whose socket is gone. guest_sce_net registers a libkernel UDP socket, waits for it to become readable and then writable, deletes it twice, closes registered sockets through close() and sceKernelClose and waits again, and checks that a descriptor that was never open still fails with EBADF. From boykopovar#11242 * fix(libkernel): retire epoll registrations of a socket closed with sceKernelClose From boykopovar#11242 * fix(libSceNet): read an option of a libkernel socket through sceNetGetsockopt Human Fall Flat (PPSA02429) opens its sockets with libkernel's socket() and reads options of them with sceNetGetsockopt. libSceNet only looked in its own socket table, so each call failed with SCE_NET_ERROR_EBADF. On the console both calls work on the same kernel sockets, the position sceNetSetsockopt already takes for MXGP 26 (PPSA30839). A descriptor libSceNet does not own but libkernel has open as a socket now goes to libkernel's getsockopt(), which answers the options it models. Anything else still fails with EBADF. The fallback runs after g_mutex is released. guest_sce_net reads SO_SNDBUF and TCP_NODELAY of a libkernel TCP socket through sceNetGetsockopt, checks that an unknown level fails with EINVAL, and that the call fails with EBADF once the socket is closed. From boykopovar#11243 * fix(libSceNet): report the receive queue length in sceNetGetSockInfo sceNetGetSockInfo always reported 0 in recv_queue_length, so a title that reads a socket only when its record shows queued data never reads. The record now carries the host's FIONREAD count; a socket the host refuses FIONREAD for keeps 0, the value a PS4 Pro on firmware 10.71 reports for an empty queue. The datagram difference between hosts and the other measured values of the record are recorded as technical debt. From boykopovar#9948 * fix(libSceNet): describe sockets made with socket() in sceNetGetSockInfo The console shares one descriptor space between the kernel socket calls and libSceNet, and sceNetGetSockInfo on a PS4 Pro (firmware 10.71) describes and lists a socket made with socket() as it does one made with sceNetSocket. AnyPS5 answered EBADF for such a descriptor and left it out of the s < 0 list. libkernel now describes one of its sockets (native handle with its owner, family, type, non-blocking mode) and lists its descriptors; sceNetGetSockInfo reads its own table first and the kernel's otherwise, without adopting the socket into libSceNet. From boykopovar#9948 * test(libSceNet): drop libkernel declarations that Api.hpp already provides --------- Co-authored-by: Josip Parlov <josipparlov@protonmail.com> Co-authored-by: ntamas94 <ntamas@live.com>
What
sceNetEpollControlandsceNetEpollWaitnow take sockets made with libkernel'ssocket(), not onlysceNetSocketones.sceNetEpollControlaccepts a descriptor that libSceNet doesn't own but libkernel has open as a socket. The libkernel check (GuestSocketIsOpen) runs outsideg_mutex. Error order is unchanged: an unknown epoll id still givesEBADF, a bad op or a missing eventEINVAL, a descriptor neither library has openEBADF, and a duplicateADDEADDRINUSE.sceNetEpollWaitlooks up the host socket of a kernel registration withKernelLookupSocketNativeafterg_mutexis released. That lookup asks libSceNet's own table (FindSocketNativeForKernel, which takesg_mutex) for descriptors libkernel doesn't own. The returned owner is held acrossWSAPoll/poll, so the host socket stays alive even if the guest closes it during the wait. The Windows closed-stream adjustment still applies only to libSceNet sockets, since it needs libSceNet's per-socket state.close()already asks libSceNet's socket closer first, andsceKernelClosenow does the same. For a descriptor libSceNet doesn't own, that closer drops the descriptor's epoll registrations, the same waysceNetSocketClosedoes at close time. So a socket that reuses the number starts unregistered. A wait still drops a kernel registration whose socket is gone, as a fallback.docs/dev/debt/libSceNet.md.Why
Human Fall Flat (PPSA02429) opens its sockets with libkernel's
socket()and registers them withsceNetEpollControl. libSceNet only looked in its own table, so everyADDandDELfailed withSCE_NET_ERROR_EBADF(0x80410109). The game retries right away, from several threads, each with its own socket.I measured it on upstream main 26ba9b6 on my test box (i5-12400F, RTX 3060, Windows): 1,674,902 failing calls in the first 30 s, half of them
DEL. A temporary counter insceNetEpollControlshowed what the descriptors are. At 1.5 M calls there were 14 descriptors (17, 18, 20 to 30, 32), each inADD/DELpairs, and every call had the same result: the epoll id was valid, the descriptor wasn't in libSceNet's table, libkernel had it open as a socket, andKernelLookupSocketNativereturned a host socket for it. There were no calls onsceNetSocketsockets.This is the same split that MXGP 26 (PPSA30839) ran into in 8b81fa0 (
sceNetSocketClose) and 3573289 (sceNetSetsockopt). I follow the position taken there: on the console both libraries work on the same kernel sockets. Epoll on libkernel sockets is not measured on a console, and the debt entry says so.Tested
CheckEpollKernelSocketinguest_sce_net, on libkernel UDP sockets:ADDreturns 0, and a secondADDgivesEADDRINUSE.sendto, the socket is reported readable, with ident and data intact.MODto writable is reported as writable.DELreturns 0, a secondDELgivesENOENT, andMODafterDELgivesENOENT.close()on one registered socket andsceKernelCloseon another, waits return 0 without a crash. ThenDELandADDon those descriptors giveEBADF.EBADF.sceKernelClose, whose number is then reused by a new socket: the new socket starts unregistered (ADDreturns 0), and a datagram sent to it isn't reported through the old registration. Without thesceKernelClosechange,guest_sce_netfails there.guest_sce_netaborts at the firstADD(0x80410109, errno 9). With the fix it passes. MinGW Release build, Windows 10.sceNetEpollControlcalls, against 1,674,902 on main.python tools/check_conventions.py --base upstream/mainpasses.Related
getsockopton libSceNet sockets). It doesn't overlap with these hunks.socket()sockets insceNetGetSockInfo. Its debt line says the other sceNet calls still answerEBADFfor such sockets, which is no longer true for epoll once this lands. There's no textual conflict.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 (none added).mdfiles or images added to the repository