Repository navigation
fix(libScePad): make the open handle tables safe for concurrent opens - #12232
Conversation
|
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.
|
|
A Windows run, since the description lists none. Head
AI-assisted: yes (Claude Code). |
|
Results in the project's real build on Windows and Linux, with controls. The description reports a throwaway archive build on Linux only. Head Windows: Windows 11 Pro 26300, AMD Ryzen 7 7800X3D, WinLibs GCC 15.2.0 r7, Ninja Release,
Linux: Ubuntu 22.04 under WSL2, GCC 11.4.0, Ninja Release, on
On both hosts the new concurrent-open check failed against the old code on its first run here. So it caught the collision deterministically on these machines, not only by luck. Not tested: ThreadSanitizer, macOS, any title. AI-assisted (Claude Code). |
|
Windows result for head Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, The tree also carried the open PRs #12290, #12288, #12285, #12269, #12265, #12206, #12201, #12199, #12197, #12165, #12146, all merged cleanly, so the run is a joint one; the tests of this PR are AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
g_opened, g_everOpened, g_remoteOpened and g_specialUsers are plain globals that scePadRead, scePadGetHandle and KnownHandle read on one guest thread while scePadOpen and scePadClose write them on another, which is a data race. The special handle slot was also claimed with a check followed by a store, so two threads opening special pads for different users could pick the same slot and one user was silently lost. The tables are atomics now and a special slot is claimed with a compare and exchange, so each concurrent open gets its own slot.
Each of 200 rounds releases eight threads together; they have to receive eight distinct handles, which are then closed.
4345f23 to
02c1c76
Compare
github-actions
Bot
added
needs rebase
# Conflicts: # core/libs/tests/GuestPad.cpp
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 364 tests)Skipped (3)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
g_opened,g_everOpened,g_remoteOpenedandg_specialUsersinlibScePad/Export.cppare plain globals.scePadRead,scePadGetHandleandKnownHandleread them on one guest thread whilescePadOpenandscePadClosewrite them on another, which is a data race. The special handle slot was also claimed with a check followed by a store, so two threads opening special pads for different users could pick the same slot and one user was silently lost.The tables are atomics now and a special slot is claimed with a compare and exchange, so each concurrent open gets its own slot.
Two commits: the fix, then the test (
CheckConcurrentSpecialOpensinGuestPad.cpp: 200 rounds in which eight threads are released together and open special pads for eight users; they must get eight distinct handles, which are then closed).Tested
Linux x86-64, a throwaway archive build with the host CPU gate patched.
guest_padpasses. The collision needs a particular interleaving, so the new test can pass on the old code by luck; it is deterministic with the compare and exchange. Not run under ThreadSanitizer.tools/check_conventions.py --base upstream/mainandgit diff --checkpass.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)