Skip to content

fix(libkernel): poll files on standard descriptors as vnodes - #10456

Merged
oneandonlydean merged 5 commits into
boykopovar:mainfrom
alnsx:codex/fix-select-redirected-input
Oct 25, 2026
Merged

oneandonlydean merged 5 commits into
boykopovar:mainfrom
alnsx:codex/fix-select-redirected-input

Conversation

@alnsx

@alnsx alnsx commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What

select and poll applied the standard-stream fallback before checking whether descriptors 0–2 referred to files. An empty file duplicated onto stdin therefore reported no readiness, even at EOF.

Check for a vnode first, so redirected files use the same readiness rules as files at any other descriptor. Tests cover all three standard descriptors, read/write readiness, exclusion of priority events, EOF and stream restoration.

This follows the existing FreeBSD model: selscan polls the file object and vop_stdpoll supplies ordinary read/write readiness. This also applies to files inherited through host redirection. Console behavior remains unmeasured.

Tested

Linux 7.2.6, GCC 13.3, Release; base e13027a1f399062d663d095ab1db139b91de0887, SPIRV-Tools disabled.

  • Both new regressions fail with the base implementation: select returns 0 instead of 2; poll returns 0 instead of 1.
  • With the fix, 8/8 focused tests pass: select, poll, filesystem, socket descriptor flags, three standard-stream cases and kqueue.
  • All libraries and the relinker rebuild successfully. Whitespace and scoped conventions checks pass; debt-length warnings remain.
  • After merging main 0119103fa9, head 7eecb44157 rebuilds all libraries/relinker and passes the same 8 focused tests (Linux, 2026-10-25). This includes the merged docs(libkernel): correct three stale socket statements #9365 documentation corrections.
  • Windows, macOS and console execution not tested; no game-unblocking claim.
ctest --test-dir build -R '^(guest_select|guest_poll|guest_filesystem|guest_standard_streams.*|guest_kqueue|guest_socket_descriptor_flags)$' -V

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 6149 docs/dev/debt/libkernel.md
PR 6740 docs/dev/debt/libkernel.md
PR 6830 docs/dev/debt/libkernel.md
PR 6936 docs/dev/debt/libkernel.md
PR 7088 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7121 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7192 docs/dev/debt/libkernel.md
PR 7201 core/libs/prx/libkernel/Socket/src/Udp.cpp
core/libs/tests/GuestPoll.cpp
PR 7447 docs/dev/debt/libkernel.md
PR 7475 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 7509 docs/dev/debt/libkernel.md
PR 7804 docs/dev/debt/libkernel.md
PR 8010 docs/dev/debt/libkernel.md
PR 8520 docs/dev/debt/libkernel.md
PR 8605 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 9156 docs/dev/debt/libkernel.md
PR 9199 docs/dev/debt/libkernel.md
PR 9242 docs/dev/debt/libkernel.md
PR 9284 docs/dev/debt/libkernel.md
PR 9293 docs/dev/debt/libkernel.md
PR 9297 docs/dev/debt/libkernel.md
PR 9302 docs/dev/debt/libkernel.md
PR 9389 docs/dev/debt/libkernel.md
PR 9509 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 9557 docs/dev/debt/libkernel.md
PR 9566 docs/dev/debt/libkernel.md
PR 9575 docs/dev/debt/libkernel.md
PR 9669 docs/dev/debt/libkernel.md
PR 9684 docs/dev/debt/libkernel.md
PR 9713 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9938 docs/dev/debt/libkernel.md
PR 9948 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 10061 docs/dev/debt/libkernel.md
PR 10442 docs/dev/debt/libkernel.md
PR 10499 docs/dev/debt/libkernel.md
PR 10501 docs/dev/debt/libkernel.md
PR 10621 core/libs/prx/libkernel/Socket/src/Udp.cpp
docs/dev/debt/libkernel.md
PR 10666 docs/dev/debt/libkernel.md
PR 10749 docs/dev/debt/libkernel.md
PR 10898 docs/dev/debt/libkernel.md
PR 11002 docs/dev/debt/libkernel.md
PR 11042 docs/dev/debt/libkernel.md
PR 11047 docs/dev/debt/libkernel.md

@harisibrar1 harisibrar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked against main 6c52b64 and FreeBSD 11.4 vop_stdpoll/selscan:

  • The reorder looks right to me. File::IsVnode only accepts regular files, directories and archived files (S_ISREG/S_ISDIR on Linux, FILE_TYPE_DISK on Windows), so a pipe or terminal on 0-2 still takes the standard-stream fallback, and the vnode readiness matches vop_stdpoll (no POLLPRI, no POLLHUP).
  • One behaviour change I'd like spelled out in the debt: IsVnode can't tell a guest dup2 from a host redirection. Launching with < file on Linux now makes fd 0 always readable, and Dungeons (PPSA16064) selects on fd 0, so it would start reading host stdin. With > log.txt, fd 1 and 2 become readable too. Main's poll entry (docs/dev/debt/libkernel.md:213) still says 0-2 follow the console model "whatever the host attaches to them", which is no longer true for files. Could that line be updated together with the select one?
  • The rewritten select entry drops two sentences that still describe current code: the /dev/random/randomdev_poll set-reporting rule and the kqueue_poll sentence. Could they be kept, with only the standard-descriptor part changed?
  • The branch merges cleanly with current main. #10501's pselect goes through select_nid_postfix, so it picks this up; #9945's NUL handles are character devices and keep the fallback (from reading only, not run on Windows).

Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 24, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 25, 2026
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 25, 2026
@alnsx

alnsx commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Updated the debt entries to cover host redirection and kept the random-device and kqueue descriptions.

#9365 is now merged and included, and this branch merges cleanly. All libraries and the relinker rebuild on Linux; all 8 focused tests pass at 7eecb44157.

Could you take another look? Could a maintainer also run the full Linux/Windows CI for this head? Windows hasn’t been tested locally.

@orezraey

Copy link
Copy Markdown
Contributor

@alnsx #11247 rewrites the Windows poll debt entry and removes the Linux one right after the entry this PR edits in docs/dev/debt/libkernel.md (stream socket resets and half-closes, now measured on a PS5), and changes poll_nid_postfix's collect step. Git may report a conflict; whichever merges second keeps both, and I'll rebase #11247 if this one lands first.

@oneandonlydean
oneandonlydean merged commit 380ffe3 into boykopovar:main Oct 25, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants