Repository navigation
fix(libSceNet): retire invalidated epoll registrations - #8598
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
|
A DEL of a socket that was closed while it was still registered now fails with const int s = sceNetSocket("x", 2, 2, 17);
sceNetEpollControl(ep, 1 /* ADD */, s, &ev);
sceNetSocketClose(s);
sceNetEpollControl(ep, 3 /* DEL */, s, nullptr); // 542c65d7: 0; this PR: 0x80410102, errno 2An ADD or MOD of the same closed id returns Suggested fix: remove the exemption, and add a close-then-DEL case to if (g_socks.count(id) == 0) return fail(NET_EBADF);A DEL of an open socket that is not registered still returns |
|
@nikowolf123 Thanks for catching this. I removed the DEL exemption in e6ec827647, so ADD, MOD and DEL now return I added |
|
@Ondra97 This no longer links on top of current Adding target_link_libraries(net_epoll_lifetime_tests PRIVATE libkernel libc libSceNetCtl)Once that's in, we'll rerun full CI. |
|
@oneandonlydean Fixed in 6ac1e70057. I added I reproduced the undefined reference before adding the link. With it, the target builds and all 30 relevant Windows runtime CTests pass without skips, including all eight epoll lifetime cases and |
|
@Ondra97 thanks for adding libSceNetCtl to the test link line, and sorry for the slow reply. The epoll changes are unchanged, and the merged files compile against current main, so I'm sending it for a full CI run. |
What
Closing a registered socket leaves its epoll entry behind. When the guest descriptor number is reused,
sceNetEpollWaitreports the replacement socket with the old registration's user data, and adding the replacement fails as a duplicate. A wait can also return an obsolete event mask or user data after a concurrent DEL, MOD or replacement registration.Close now removes the descriptor from every epoll under the existing mutex. Registrations are immutable snapshots with distinct identities. After the host poll, wait checks that each registration is still current before copying its event under the same mutex. Native socket handles remain pinned through the host poll. ADD, MOD and DEL all reject closed or invalid descriptors with
EBADF; DEL of an open, unregistered socket continues to returnENOENT, following Linux epoll_ctl(2). Console error ordering is unmeasured and recorded in the debt.Eight regression cases cover reuse across two epolls, control errors for open/unregistered, closed and never-opened descriptors, close during wait, DEL, MOD, DEL followed by ADD, abort and destroy. A test-only barrier pauses after a real loopback readiness result; the test then completes the control operation before releasing the captured result. Each fixed case runs three rounds.
Descriptor identity and close cleanup follow Linux epoll(7). Console close/control ordering is unmeasured and is recorded in the library debt, together with the existing lack of duplicate-descriptor alias lifetime support. This addresses ON C1 in #8005; no game compatibility claim is made.
Tested
Windows 11 Pro 10.0.26300; AMD Ryzen 9 7900X, 32 GB RAM, NVIDIA GeForce RTX 4070 Ti SUPER. WinLibs GCC 15.2.0 UCRT r7; Release,
BUILD_TESTING=ON.Current main base:
7d7b097407a0d5d1ddd243d98f0ddc5d9cbc5420. Tested head:05d3fe38c004d59dbe5737f29add23161b6f3f29.The branch now includes the current main socket transport from #8653. The only manual merge conflict was in
docs/dev/debt/libSceNet.md; I retained both the updated transfer debt and the epoll lifetime entry. The production epoll patch is identical to the previous head apart from line offsets and blob IDs. The explicitlibSceNetCtllink added after #8268 remains in place. A new maintainer-triggered full CI run is needed for this head.The original lifetime comparison below used base
542c65d74c95bd410f05dc70ec310d06c3fbdd36and the original PR headc7032e6acf8c1e5c32e3be15e6b636d923cabea9; those measurements are retained from the initial validation.The unchanged
guest_sce_nettest and the abort/destroy cases pass before and after. The five failures on base are reuse, in-flight close, DEL, MOD and replacement registration. The CTest baseline was built with the unmodified baseExport.cppand the new regression.For the review regression, the new
net_epoll_lifetime_controlCTest fails on the original PR head at the close-then-DEL assertion. After removing the DEL exemption, that test and the other eight focused tests pass. Each control run also checks ADD/MOD on closed and never-opened descriptors and preservesENOENTfor DEL on an open, unregistered socket. The current head passes all 43 relevant runtime CTests without skips: all eight epoll lifetime cases with three rounds each,guest_sce_net,guest_net_ctl, existing datagram and guest-buffer tests, all elevennet_dontwait_*cases from current main, both SSL transport providers, and patched-library loading. All 354 patched libraries load with zero failures.#8653 is now merged, and its socket transport and updated debt are preserved. The overlap checker reports only incomplete comparisons against older conflicting PRs. I reviewed the incomplete comparisons: #5836 changes peer transport support, #6879 adds a platform include, #7201 adds platform/DNS handling, #8204 adds Ethernet text conversion, and #6044 reorganizes existing tests. None implements this fix.
No console module or game payload is needed by the tests. No game run was performed.
The full Windows runtime build passed on this head: the relinker, every system library and patched library, and all executables used by the focused test command above. This build used the submitted source without a temporary overlay.
The final main sync also includes #8543.
python tools/tests/test_unwrap_self.pypasses all 32 tests on this head.The earlier all-target build at
6ac1e700578c0562bf7bcb881d18add727d374f1required the test-only #8310 fix, exact headca55e46f98ad85861dd7567abad0c028274daca1, forcore/libs/prx/libSceAgcDriver/tests/execution/ImageArrayWindowPastLayerLimit.cpp:33. Its unchanged main version usesalignas(65536), which exceeds the supported MinGW object-file limit of 8192 (blob0d29ae08e46e33c1cd0e931f951befcf42961ec5). #8310 remains an open dependency for that all-target build. The current validation builds the full runtime and focused test targets listed above; it does not claim a new all-target test-executable build.python tools/check_conventions.py --base origin/main,python tools/check_test_registration.py ../build,python tools/check_main_sync.py --base origin/main --strictandgit diff --checkpass on this head. The registration audit reports 1146 test executables with none unregistered.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)