Skip to content

fix(libSceNetCtl): keep the callback argument and deliver the state from sceNetCtlCheckCallback - #6138

Open
fdfawef324 wants to merge 1 commit into
boykopovar:mainfrom
fdfawef324:claude/mc-netctl-check-callback
Open

fdfawef324 wants to merge 1 commit into
boykopovar:mainfrom
fdfawef324:claude/mc-netctl-check-callback

Conversation

@fdfawef324

Copy link
Copy Markdown
Contributor

What

Stop 2 of the Minecraft Preview bring-up (#1277), NetCtl half: "NetCtl must store the callback argument and deliver the state from sceNetCtlCheckCallback". On main, sceNetCtlRegisterCallback and sceNetCtlRegisterCallbackV6 store only the function and drop arg, and sceNetCtlCheckCallback returns 0 without calling anything, so a title that polls sceNetCtlCheckCallback until its callback reports the connection state never gets it.

  • NetCtlCallback in SceTypes.hpp is now void (APS5_VABI *)(int, void*). Without APS5_VABI, the Windows build would call the guest function with the Microsoft ABI. The captureless lambda in GuestSceNet.cpp no longer converts to it, so it becomes an APS5_VABI function.
  • Both callback tables hold slots {func, arg, lastEvent}. Registration stores func and arg in an empty slot and writes *cid; unregistration clears the slot. Error codes, and the result for an empty slot, are unchanged.
  • With no sceNetCtlRegisterCallback callback registered, sceNetCtlCheckCallback returns 0 without querying the host.
  • Otherwise it takes the event from the predicate sceNetCtlGetState uses: IPOBTAINED (3) while the host has a non-loopback address, DISCONNECTED (1) otherwise. The values come from shadPS4 net_ctl_codes.h L27-29. Under the lock it marks the IPv4 slots whose lastEvent differs. After the lock is released, it calls each of them on the calling thread as func(event, arg), so a callback can unregister itself without deadlocking (tested).
  • Each slot gets an event once per state change, as in KytyPS5 network.cpp @9e41d832 (NetCtlCallbackSlot L3447-3451, NetCtlCheckCallback L3648-3661, NetCtlRegisterCallback L3687-3707). shadPS4 net_ctl_obj.cpp L47-57 delivers the state on every check instead. Neither model is measured on hardware, and the debt entry records the difference.
  • KytyPS5 calls the callbacks under its lock, so its NetCtlUnregisterCallback waits for a running check. Here they are called from a copy of their slots, so an unregister or sceNetCtlTerm on another thread that returns while a check is in flight does not stop that check from calling the callback once more with its old argument. The debt entry records this.
  • sceNetCtlTerm clears both tables, as KytyPS5 NetCtlTerm (L3625-3633) does.
  • V6 slots keep their argument but get no event, because no source says which call delivers them. This is recorded in the debt entry.

A Functional entry in docs/dev/debt/libSceNetCtl.md records that the delivery model comes from KytyPS5 and is not measured on hardware, that the state comes from the host interfaces, the in-flight call after a concurrent unregister, and that V6 callbacks get no event.

Cost: on Windows the host query is two GetAdaptersAddresses calls, the same as one sceNetCtlGetState on main. On this host a check with a callback registered costs about 8.6 ms when the host is idle and 10-23 ms while parallel builds run; sceNetCtlGetState costs the same. With no callback registered, a check returns before the query (under 1 us). A follow-up PR will cache the predicate on Windows (invalidated by NotifyUnicastIpAddressChange/NotifyIpInterfaceChange, or built on GetUnicastIpAddressTable) and retry GetAdaptersAddresses on ERROR_BUFFER_OVERFLOW. Until then, an address added between the size query and the fetch reads as no address, as it already does for sceNetCtlGetState, and here it delivers DISCONNECTED and then IPOBTAINED on the next check.

This covers only the NetCtl half of stop 2. The NpManager half (state callbacks and SIGNED_OUT from sceNpCheckCallback) is a separate change. Minecraft Bedrock (PPSA17221) is playable on main without this change (#5744), so the change was not needed there. The Preview needs it for stop 2 of #1277. No open PR covers these functions; the last change to them was #5427 (RegisterCallbackV6/UnregisterCallbackV6). #6039 also appends to docs/dev/debt/libSceNetCtl.md; whichever merges second resolves a two-line append.

Test: guest_net_ctl (core/libs/tests/GuestNetCtl.cpp) now runs callback cases before its existing exit 77 for a host without an IPv4 address. The expected event comes from sceNetCtlGetState, so the result does not depend on the host's connectivity.

  • A V6 callback is registered before the first check and again after sceNetCtlTerm (cid 0 both times); it is never called.
  • After a register with &probe and a check, the callback has been called exactly once, with arg == &probe and the expected event. A second check calls nothing.
  • A second callback with a different argument: the next check calls only the new one, with its own argument.
  • A callback that is registered and then unregistered before a check is not called.
  • A callback that unregisters its own cid from inside the call: the unregister returns 0, the check returns, the next check does not call it, and the next registration reuses its cid.
  • A callback registered before sceNetCtlTerm is not called by a check after it. Registering again after Term gets cid 0, and the next check delivers the current state again.

Tested

Windows Server 2025 (10.0.26100), WinLibs MinGW-w64 GCC 15.2.0, Ninja, Release, BUILD_TESTING=ON, on main eea428c.

Not tested: Linux and macOS (there APS5_VABI is empty and the test fails at runtime instead of at compile time), and any title. I did not run Minecraft Preview.

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
  • Unblocks: PPSA19639 Minecraft Preview (PPSA19639) #1277
  • GPUs tested: n/a (no AGC, shader recompiler or VideoOut change)
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

…rom sceNetCtlCheckCallback

sceNetCtlRegisterCallback and sceNetCtlRegisterCallbackV6 stored only
the function and dropped its argument, and sceNetCtlCheckCallback
returned 0 without calling anything, so a title that polls
sceNetCtlCheckCallback until its callback reports IPOBTAINED never
got the event.

Each callback slot now keeps the function, its argument and the last
event it received; unregistration clears the slot. With no IPv4
callback registered, sceNetCtlCheckCallback returns 0 without querying
the host. Otherwise it takes the state from the same host-interface
predicate as sceNetCtlGetState, IPOBTAINED (3) or DISCONNECTED (1)
(shadPS4 src/core/libraries/network/net_ctl_codes.h L27-29
@15abf68c9b52), marks the IPv4 slots whose last event differs and
calls them on the calling thread with their argument after the lock is
released, so a callback can unregister itself. Delivery once per state
change and slot follows KytyPS5 src/libs/network.cpp @9e41d8320a27
(NetCtlCallbackSlot L3447-3451, NetCtlCheckCallback L3648-3661,
NetCtlRegisterCallback L3687-3707); shadPS4 net_ctl_obj.cpp L47-57
delivers on every check instead, and the debt entry records the
difference and that KytyPS5 calls the callbacks under its lock.
sceNetCtlTerm clears both tables, as KytyPS5 NetCtlTerm (L3625-3633)
does. V6 slots keep their argument but get no event, since no source
says which call delivers them.

NetCtlCallback is now an APS5_VABI pointer, so the Windows build calls
the guest function with the System V ABI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 14, 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 5836 core/libs/tests/GuestSceNet.cpp
PR 5965 core/libs/SceTypes.hpp
PR 6023 core/libs/SceTypes.hpp
PR 6039 docs/dev/debt/libSceNetCtl.md
PR 6044 core/libs/tests/GuestNetCtl.cpp
core/libs/tests/GuestSceNet.cpp
PR 6140 core/libs/tests/GuestSceNet.cpp
PR 6142 core/libs/tests/GuestSceNet.cpp
PR 6170 core/libs/SceTypes.hpp
PR 6494 core/libs/tests/GuestSceNet.cpp
PR 6685 core/libs/prx/libSceNetCtl/Export.cpp
core/libs/tests/GuestNetCtl.cpp
docs/dev/debt/libSceNetCtl.md
PR 6690 core/libs/tests/GuestSceNet.cpp
PR 6691 core/libs/tests/GuestNetCtl.cpp
PR 6720 core/libs/prx/libSceNetCtl/Export.cpp
core/libs/tests/GuestNetCtl.cpp
docs/dev/debt/libSceNetCtl.md
PR 7081 core/libs/SceTypes.hpp
PR 7201 core/libs/prx/libSceNetCtl/Export.cpp

@isnisk

isnisk commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Checked on Linux: Ubuntu 26.04 (WSL2), GCC 15.2.0, Release, this head (a5e4164ed) merged onto main 489466d3f (clean merge).

  • With the change: guest_net_ctl, guest_sce_net and guest_netctl_ap_dialog pass, 3 rounds; no warnings from the changed files.
  • Without it (main's Export.cpp, the new test and the NetCtlCallback type change kept): guest_net_ctl aborts 3 of 3. As you expected, on Linux it compiles and fails at runtime instead of at compile time.
  • check_conventions.py --base origin/main passes.

Not checked: macOS, the title.

AI-assisted: yes (Claude Code).

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the diff against KytyPS5 NetCtlCheckCallback/RegisterCallback and the debt entry, and built and ran guest_net_ctl + guest_sce_net on the merged tree (35/35 in scope pass; guest_net_ctl fails with the non-test changes reverted, matching isnisk's Linux run). Callbacks run outside the lock, so the self-unregister case is covered; merging once the Windows build is green.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 17, 2026

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

Labels

docs needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants