Repository navigation
fix(libkernel): report a stream whose peer closed as readable in poll on Windows - #9492
EagleBearerYS wants to merge 2 commits into
Conversation
… on Windows When the peer of a stream socket closes, Winsock's WSAPoll reports POLLHUP without POLLRDNORM, whether or not the socket's own sending side is shut down. poll passed that on, so on Windows a guest socket whose peer had closed came back as POLLHUP alone and never as POLLIN. FreeBSD's sopoll_generic reports a socket that can receive no more as readable, and adds POLLHUP only once it can send no more either. Linux does the same, so the Linux path already behaved this way. On Windows, a stream socket's POLLHUP without POLLERR now reports POLLRDNORM when reading was asked for, and keeps POLLHUP only after shutdown has shut the socket's own sending side, as recorded in Socket::sendShut. sceNetEpollWait gets the same treatment in boykopovar#5376.
On Windows a socketpair stream end used to report POLLHUP without POLLIN after its peer's shutdown(SHUT_WR) or close. With the previous commit it reports POLLIN, as the console does after a shutdown. After a close the console also reports POLLHUP, which the loopback connection behind the pair cannot tell from a shutdown.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
This PR implements the same Windows poll behavior as the still-open #8483, and the two PRs change the same Udp.cpp poll path. The body already identifies #8483 and says only one should go in. Please resolve that overlap with the other contributor and maintainers, then update or close one PR before this duplicate work proceeds. I have not assessed which implementation should be selected; both should not be merged.
|
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.
|
|
Thanks for raising the duplicate with #8483. I still do not see an agreement from that PR author or maintainers on which implementation will proceed, so the duplicate remains unresolved. Also, #7605 edits the same |
|
Thanks, @MarcZX10. Depends on now lists #7605, and the Overlap section says this PR goes after it and rebases on it. On #8483, the question of which one stays is with its author and the maintainers. Once it's settled, the other closes, and the surviving branch gets its checklist updated and is brought up to date with current |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The Windows poll translation is scoped to stream POLLHUP without POLLERR, and its regression checks half-close, peer close/EOF, and the both-directions-shut state. The PR correctly records #7605 as a prerequisite and includes Fixes: #8482; #8482 remains open. However, duplicate work #8483 is also still open and implements the same behavior. The PR itself says one should close, but there is still no author/maintainer decision. Please resolve that choice with #8483’s author and maintainers, update/close the superseded PR, and preserve the #7605 order. This branch and its tests are based on a6dcea9, while current upstream main is ab347d1; rebase the surviving PR and rerun guest_poll, conventions and overlap checks on current main. Also preserve the #8344/#8483/#8670 debt edits and manually compare #5021’s Udp.cpp warning. Hosted product jobs are skipped; I did not run local tests.
github-actions
Bot
added
libkernel
docs
needs rebase
|
Closing this one: #8483 landed the same change to |
Opened again from #5403: title-less conformance fixes of this kind are merged now, for example #7365 and #8411.
What
When the peer of a stream socket closes, Winsock's
WSAPollreportsPOLLHUPwithoutPOLLRDNORM, whether or not the socket's own sending side is shut down.pollpasses that on, so on Windows a guest socket whose peer has closed comes back asPOLLHUPalone and never asPOLLIN.FreeBSD's
sopoll_genericreports a socket that can receive no more as readable, and addsPOLLHUPonly once it can send no more either. Linux'spolldoes the same, so the Linux path already behaves this way.On Windows, a stream socket's
POLLHUPwithoutPOLLERRnow reportsPOLLRDNORMwhen reading was asked for. It keepsPOLLHUPonly aftershutdownhas shut the socket's own sending side, theSocket::sendShutstate #4903 added. The two commits:socketpairnote indocs/dev/debt/libkernel.md. Its stream ends are loopback TCP connections, so after the peer'sshutdown(SHUT_WR)they now reportPOLLIN, as the console does. After the peer'sclosethe console also reportsPOLLHUP, which the loopback connection cannot tell apart from a shutdown.polllooks the socket up by descriptor, and only when it reports a hang-up without an error. A libSceNet socket waited on throughpoll(#4933) keeps the flagsWSAPollreports.Unchanged:
POLLHUPwithPOLLERR) keepsWSAPoll's flags.selectalready counts a hang-up as readable.No title is claimed.
Overlap: #8483 makes the same
pollchange, inpoll'scollectstep, with a newpolldebt entry. #5403, where this change was first opened on 2026-10-13, was closed on 10-14 for having no title. Either can go in; the other then closes.#7605 changes the same
pollloop for kqueues, next to the line this PR adds afterWSAPoll, and the two compose. Merge order: this PR goes after #7605 (merge order only) and rebases on it.Tested
On GitHub's runners from my fork, on
mainat a6dcea9 (run, of the same tree as these two commits):guest_pollgains three checks:POLLINpoll of the client reports nothing;POLLIN | POLLOUT;POLLINpoll reportsPOLLIN | POLLHUP.Results:
main'sUdp.cppthe test aborts. With the change,guest_pollpasses 10 runs in a row, and every test registered undercore/libs/prx/libc(142) andcore/libs/prx/libkernel(207) passes.guest_pollpasses withmain's code and, 10 runs in a row, with the change. The libkernel suite (197) passes. In libc (115), the timing testprecise_waitfailed with and without the change in this run.tools/check_conventions.py --base origin/mainpasses for both commits.Checklist
main; fix(libkernel): report a stream whose peer closed as readable in poll on Windows #8483 makes the same change (see Overlap)NotImplemented_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)