Repository navigation
feat(libkernel): implement pselect over select - #9273
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.
|
|
Independent Windows run of this PR (data only). Environment: Windows 10 Pro 10.0.19045, Ryzen 7 5800X3D, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release, BUILD_TESTING=ON. Merge-tree of the PR head against PR head tested: Results (3 runs each):
Not run: other platforms, the other tests in the same family, the full repository CTest, and any title or console run. AI-assisted (Claude Code). |
The PS5 3.20 export lists have pselect in libkernel as well as libkernel_sys, and on Windows a libkernel import never reaches libkernel_sys. The implementation moves unchanged to libkernel/Socket/src/Pselect.cpp, beside select.
|
Linux: Ubuntu 24.04.1 LTS (WSL2), GCC 13.3.0, Release; head
Run from the PC (Windows host, Linux in WSL2), not from the PR author's machine. |
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main a6646f8 and FreeBSD release/11.4.0 sys/kern/sys_generic.c (sys_pselect/kern_pselect, kern_select):
- The timespec handling matches
TIMESPEC_TO_TIMEVAL(truncation, also for small negativetv_nsec). TheEINVALcases fortv_nsec >= 1000000000andnfds < 0matchkern_select's checks, and the mask is restored witherrnokept. Thanks for the clear tests. - One gap seems larger than the debt line says. On main,
select_nid_postfix(Socket.cpp) has noEINTRpath; it waits until a descriptor is ready or the timeout ends. And_sigprocmask(Signals.cpp:583-586) runs the handler of a pending signal thatSIG_SETMASKunblocks synchronously, inside the call. So by my reading the usual pselect idiom doesn't interrupt the wait: blockSIGUSR1,raise(SIGUSR1), thenpselect(0, NULL, NULL, NULL, &{0, 300000000}, &empty). FreeBSD returns -1/EINTRat once (kern_selectsleeps withPCATCH). Here the handler runs insidesigprocmask, thenselectwaits the full 300 ms and returns 0, and with a NULL timeout it wouldn't return. Apthread_killfrom another thread during the wait only marks the signal pending, so it doesn't end the wait either. I didn't get to run a probe for this, so it's from reading the code. - Could pselect return -1/
EINTRwhensigmaskunblocks a signal that is already pending?sigpendingbefore the swap gives that set: let the swap deliver it, restore the mask and skip the wait. That would cover the idiom above. - The debt line could then say that a signal raised during the wait never interrupts it, because
selecthas noEINTRpath. It could also say that the mask swapped here is the process-wide one, so it applies to every thread for the duration of the wait.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
…elect Moved our pselect (same select/sigprocmask approach, but it throws when restoring the mask fails where the PR ignores it) from libkernel_sys/Export.cpp into the PR's libkernel/Socket/src/Pselect.cpp so libkernel exports it, with our debt line in place of the PR's; dropped the PR's test (GuestKernelSys CheckTimeAndSelect covers pselect).
What
pselectwas a discovery stub inlibkernel_systhat threwNotImplemented. It is now implemented in libkernel, inlibkernel/Socket/src/Pselect.cppbesideselect, and the libkernel_sys stub is removed. It is built on libkernel'sselectandsigprocmask, as FreeBSD 11sys_pselect/kern_pselect(sys/kern/sys_generic.c) do:TIMESPEC_TO_TIMEVAL's truncation (tv_usec = tv_nsec / 1000), andselect's own timeval check rejects what FreeBSD'sitimerfixrejects. Sotv_nsecof 1,000,000,000 or more givesEINVAL, 999,999,999 is accepted, andnfds < 0givesEINVAL.sigmaskis installed withSIG_SETMASKbefore the wait and the previous mask is restored after it, keepingselect'serrno. Asigprocmaskfailure returns -1 before any wait.Moved to libkernel so libkernel imports reach it on Windows. The PS5 3.20 export lists have
pselectin libkernel as well as libkernel_sys, and on Windows a libkernel import never searches libkernel_sys (WindowsEntryStubBuilder.cpp:175). With the libkernel export and no libkernel_sys stub, both imports reach it, and the duplicate-export rule holds. The code is unchanged by the move.A debt line in
libkernel.mdrecords that the mask swap is not atomic with the wait: a signal that becomes deliverable between the swap and the wait is not guaranteed to interrupt it.Tested
Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, Release. This is a CPU-side export, so no GPU is involved.
guest_kernel_sys_pselect(core/libs/tests/GuestKernelSysPselect.cpp), on a pipe:SIGUSR1blocked throughsigmaskand the original mask back afterwards;tv_nsec999,999,999 and with a NULL timeout;tv_nsec1,000,000,000 givesEINVALwith the mask restored;nfds-1 givesEINVAL.mainthe test aborts withpselect_nid_postfix not implemented. On this branch it passes, 3 of 3 runs.guest_kernel_sysstill passes, andtools/check_conventions.py --base origin/mainpasses.Pselect.cppremoved from libkernel's sources, the test fails to link (undefined reference to pselect_nid_postfix), so it reaches libkernel's export.pselectis exported by libkernel only.tools/pr_overlap.pycrashes on this machine's partial clone, so I rangit merge-treeagainst every open PR that touches these files. Removing the libkernel_sys stub conflicts textually with feat(libkernel_sys): identify the initial pthread #9160 and with my feat(libkernel): implement four pthread_attr forms #9156, which edit the neighbouring stubs. That's a one-line resolution, by mergingmainafter either lands. Everything else merges cleanly.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)