Skip to content

fix(libkernel): treat a socket direction that is already shut as shut on macOS hosts - #9625

Open
rooklz wants to merge 1 commit into
boykopovar:mainfrom
rooklz:fix/macos-shutdown-already-shut
Open

rooklz wants to merge 1 commit into
boykopovar:mainfrom
rooklz:fix/macos-shutdown-already-shut

Conversation

@rooklz

@rooklz rooklz commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

guest_socket_pair fails on macOS hosts at GuestSocketPair.cpp:238: after the peer's shutdown(SHUT_WR), shutdown(pair[1], SHUT_RD) returns -1 with ENOTCONN. A small host program shows the same thing: on a connected AF_UNIX stream pair, shutdown(b, SHUT_RD) succeeds on a fresh pair, fails with ENOTCONN (57) once the peer has shut its sending side, and the 5 queued bytes are still readable afterwards.

The two kernels differ here:

  • XNU's soshutdownlock_final (bsd/kern/uipc_socket.c) returns ENOTCONN when the direction asked for is already shut (/* read already shut down */, /* write already shut down */), and for SHUT_RDWR it jumps to done before the write side when the read side was already shut.
  • FreeBSD 11.4's soshutdown (sys/kern/uipc_socket.c) returns ENOTCONN only when the socket isn't connected; for a connected socket it shuts what was asked and returns 0, whatever was shut before.

On macOS, shutdown now treats ENOTCONN from a socket that still has a peer (getpeername succeeds) as a direction that was already shut: the call succeeds and the guest's shut state is recorded as before. For SHUT_RDWR the write side is then shut explicitly, since XNU skipped it. A socket without a peer still fails with ENOTCONN, as on FreeBSD. Linux and Windows behave as before.

@iamthehimansh's #8658 and @marion909's #7201 make the same change as part of their macOS ports (#8658 for SHUT_RD only). This is that change on its own with the test it fixes, as #8920 did for the fflush guard. Whichever lands first, the other drops these lines.

Tested

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

@rooklz rooklz mentioned this pull request Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 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 5021 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 7088 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 7201 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 7502 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 7511 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 8605 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9509 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9515 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9578 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9713 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 9948 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 10456 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 10621 core/libs/prx/libkernel/Socket/src/Udp.cpp
PR 10767 core/libs/prx/libkernel/Socket/src/Udp.cpp

@MarcZX10 MarcZX10 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.

The macOS behavior is narrowly scoped: an ENOTCONN result is treated as an already-shut direction only when getpeername confirms the socket still has a peer; SHUT_RDWR explicitly attempts the write shutdown after XNU’s read-side early exit. The reported macOS control (same integration tree without this change fails, with it passes) and Linux test pass are useful, but the hosted product jobs are skipped and I did not rerun runtime tests here. Exact-head conventions and whitespace pass. Please coordinate the shared Udp.cpp hunk with #8658 and #7201 before merge; the description says either port PR must drop these lines if this one lands first, so record the agreed order/dependency in the checklist or thread.

I’ve noticed that holding this PR because of #5239 isn’t necessary. It’s fine for PRs to keep coming in; they’ll be handled in due course. Please disregard my earlier request to hold or defer it.

@rooklz

rooklz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

On the #5239 direction: this came from running the suite on the macOS port tree discussed on #1, where the shutdown tests fail without it, not from a title. I am fine holding it until the queue drops or a maintainer says otherwise, or closing it in favour of #8658 if that is preferred.

Merge order, recorded here: if #8658 or #7201 lands first, I rebase this and drop whatever their Udp.cpp hunk already covers. If this lands first, their hunk at those lines conflicts and the resolution is to keep this version of shutdown_nid_postfix.

@MarcZX10

MarcZX10 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

I rechecked the exact PR head (a17017132dcb90d0b96c8dedb7fe3d63f728e20f) against current main (8b3a6ba88d8fc74e768f1a7e8e9e36fb69842768). git diff --check and tools/check_conventions.py --base <current-main> --head HEAD pass. The focused overlap scanner did not finish in this environment, so I am not treating its result as evidence; the existing overlap report and manual diff show this PR changes shutdown_nid_postfix in Udp.cpp, while #8658 and #7201 are also open and concern macOS shutdown behavior. Your proposed merge-order resolution is useful, but I could not find confirmation from those PR authors. Please rebase onto current main and record the agreed landing order / exact duplicate-hunk handling with the affected authors before merge.this is feedback on the stale base and unresolved shared-file coordination only. The repository product build and macOS runtime test were not run here; the previously reported macOS red/green and Linux test evidence remains author-provided.

@rooklz
rooklz force-pushed the fix/macos-shutdown-already-shut branch from a170171 to 7c21378 Compare October 22, 2026 13:29
@rooklz rooklz mentioned this pull request Oct 22, 2026
8 of 11 tasks
@rooklz

rooklz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (8b3e39832a): head 7c213787ac, same diff. check_conventions.py passes on it.

Refreshed results on this head: guest_socket_pair passes 3 of 3 on the macOS integration tree (the same tree as in the description, carrying exactly this Udp.cpp), and a hosted Linux run of the socket tests is at https://github.com/rooklz/AnyPS5/actions/runs/39787396014 (in progress; I'll note the result).

Landing order with the two port PRs is now written on both of them, #8658 and #7201: whichever lands first, the other drops its copy of the shutdown_nid_postfix hunk, and if a port lands first I rebase this onto it. I've asked both authors to say if they'd rather fold it in, in which case I close this.

@rooklz

rooklz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

The hosted Linux run on head 7c213787ac finished: the socket tests, guest_socket_pair among them, pass (https://github.com/rooklz/AnyPS5/actions/runs/39787396014, Ubuntu 24.04, GCC, Release).

@MarcZX10

Copy link
Copy Markdown
Contributor

I’ve noticed that asking you to hold this PR was unnecessary. It’s fine to keep submitting PRs; maintainers will handle them in due course.

@maximilianfv

Copy link
Copy Markdown
Contributor

Independent test: Apple M1 Pro, macOS 27.0.1

Apple M1 Pro, macOS 27.0.1, Apple clang 21, x86-64 Release build under Rosetta 2, BUILD_TESTING=ON, no Vulkan loader. Base: mugurc's ci/macos-series-2 at 27e968974, with this pull request's own commits cherry-picked together with #9971, #9972, #8920, #9509, #9625, #9231 and #9733.

  • guest_socket_pair: aborts at GuestSocketPair.cpp:238 without this change, as in your report, and passes with it in 2 of 2 runs. So the fix also holds on macOS 27, beyond the 26.2 run in the description.

Full ctest -j 6 (mic tests excluded): no test that passed before fails with this batch applied. precise_wait and guest_refused_connect are flaky here either way.

AI-assisted: yes (Claude Code ran the build and the tests).

@rooklz

rooklz commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@maximilianfv thanks for the M1 Pro run on macOS 27; the description's Tested section now lists it. That covers both macOS majors the series builds on.

This branch has not been deployed

No deployments
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.

3 participants