Skip to content

fix(libkernel): read and write the detached state of a guest thread atomically - #10810

Merged
oneandonlydean merged 5 commits into
boykopovar:mainfrom
MarcZX10:marc/fix-thread-detached-race
Nov 2, 2026
Merged

oneandonlydean merged 5 commits into
boykopovar:mainfrom
MarcZX10:marc/fix-thread-detached-race

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What

PthreadPrivate::_detached was a plain bool. scePthreadDetach writes it under the thread's join mutex, while scePthreadAttrGet, pthread_attr_get_np and the first check of the join path read it with no lock or under a different one. ThreadSanitizer reports this as a data race between scePthreadDetach and scePthreadAttrGet (and between detach and join).

The field is now std::atomic<bool>. All existing reads and writes compile unchanged and behave as before; no other change.

Tested

Linux x86-64, ThreadSanitizer build (-fsanitize=thread, RelWithDebInfo, dynamic libstdc++ because TSan cannot link statically). The host CPU gate was bypassed locally for the run only (not committed). No Windows, no ASan/UBSan run for this change, no console hardware.

  • The existing guest_pthread_stack test already reported the race at Attr.cpp:97 (read) against Thread.cpp:1108 (write) on current main.
  • New guest_pthread_detach_attr queries the attributes of a blocked thread from a second thread while the first detaches it, 50 rounds, and checks the attributes report the detached state afterwards. On current main under TSan it reports data race ... in scePthreadDetach; with the fix TSan reports nothing in three runs, and guest_pthread_stack no longer reports it. guest_pthread_join_detach passes.
  • The new test passes without a sanitizer both before and after the fix: the defect is only observable with a race detector, so it is a regression harness for TSan runs.

This PR is stacked on #10816. Rebase follow-up (2026-10-25): it now follows #10816 head 8eaa2bbad58d... on upstream main a5a780eaaa7f187fe8f5c7f2ef244f6a6dab2762, without conflicts. Both dependency test registrations remain in libkernel/CMakeLists.txt. Strict GCC C++20 syntax-only checks with -Wall -Wextra -Werror passed for Thread.cpp (suppressing its existing missing-field-initializer warning) and GuestPthreadDetachAttr.cpp. git diff --check upstream/main...HEAD and python3 tools/check_conventions.py --base upstream/main --head HEAD passed. The full local CMake test build could not configure because the SDL2 CMake package is unavailable, so no post-rebase runtime or ThreadSanitizer result is claimed. The reported runtime evidence is from the prior base.

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 (none added)
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: fix(libkernel): return ESRCH from thread name, priority and affinity calls on a released thread handle #10816
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

Rebase validation (2026-10-25)

This PR is stacked on updated #10816 head 8eaa2bbad5; #10816 is based on upstream main a5a780eaaa7f187fe8f5c7f2ef244f6a6dab2762. The rebase completed without conflicts, and both dependency test registrations remain present. git diff --check upstream/main...HEAD and python3 tools/check_conventions.py --base upstream/main --head HEAD passed. Full local CMake configuration could not proceed because the SDL2 package is unavailable; no post-rebase runtime or ThreadSanitizer result is claimed.

@github-actions

github-actions Bot commented Oct 25, 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 3986 core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 4563 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 4630 docs/dev/debt/libkernel.md
PR 5021 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
docs/dev/debt/libkernel.md
PR 6936 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 7088 docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7121 docs/dev/debt/libkernel.md
PR 7135 docs/dev/debt/libkernel.md
PR 7149 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
docs/dev/debt/libkernel.md
PR 7192 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 7447 docs/dev/debt/libkernel.md
PR 7464 docs/dev/debt/libkernel.md
PR 7804 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
docs/dev/debt/libkernel.md
PR 8592 core/libs/prx/libkernel/Pthread/include/Pthread.hpp
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 9019 core/libs/prx/libkernel/CMakeLists.txt
PR 9156 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9199 docs/dev/debt/libkernel.md
PR 9293 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9297 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9302 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9383 core/libs/prx/libkernel/CMakeLists.txt
PR 9389 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9557 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9566 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 9843 docs/dev/debt/libkernel.md
PR 10025 docs/dev/debt/libkernel.md
PR 10319 docs/dev/debt/libkernel.md
PR 10442 docs/dev/debt/libkernel.md
PR 10558 docs/dev/debt/libkernel.md
PR 11831 core/libs/prx/libkernel/CMakeLists.txt
PR 11939 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 12544 core/libs/prx/libkernel/CMakeLists.txt
core/libs/prx/libkernel/Pthread/src/Thread.cpp
PR 12560 docs/dev/debt/libkernel.md
PR 13582 core/libs/prx/libkernel/Pthread/src/Thread.cpp
docs/dev/debt/libkernel.md
PR 13587 docs/dev/debt/libkernel.md
PR 14103 docs/dev/debt/libkernel.md
PR 14128 docs/dev/debt/libkernel.md
PR 14212 docs/dev/debt/libkernel.md
PR 14264 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 14299 core/libs/prx/libkernel/CMakeLists.txt
PR 14497 docs/dev/debt/libkernel.md

@MarcZX10

MarcZX10 commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebase follow-up: head 3b1041f444c564e7c1729a313869972f2084bc2a now follows rebased #10816 (607f6550ec8c) on upstream main e5f23554c9c7b1929f1ef7a65f6ea33a24fd189e. The #10810 commits retain MarcZX10 as author and committer. In the complete rebased stack, Thread.cpp and the #10816/#10810/#10817 pthread test sources pass GCC 12.2 C++20 syntax checks with -Wall -Wextra -Werror (suppressing the existing missing-field-initializer warning); conventions and git diff --check pass at this head. Runtime/ASan tests were not rerun because local CMake configuration is blocked by the unpopulated SDL2 submodule and missing XRandR development support. Hosted checks are restarting.

@MarcZX10
MarcZX10 force-pushed the marc/fix-thread-detached-race branch 6 times, most recently from a6aee5f to b79959c Compare October 25, 2026 11:47
@github-actions github-actions Bot added the docs label Oct 25, 2026
@wachi35676

Copy link
Copy Markdown
Contributor

Windows run of head b79959c5e3af merged onto main 90f918fa7. WinLibs GCC 15.2.0, Release.

  • guest_pthread_detach_attr and guest_pthread_handle_after_exit, --repeat until-fail:15: 30/30 pass.
  • With main's Pthread.hpp/Thread.cpp, guest_pthread_handle_after_exit fails (0xc0000409) and guest_pthread_detach_attr still passes.

AI-assisted: yes (Claude Code); results above were run on this machine.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 27, 2026
@MarcZX10
MarcZX10 force-pushed the marc/fix-thread-detached-race branch from b79959c to 2567826 Compare October 27, 2026 04:23
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 27, 2026

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

ThreadSanitizer run of head 256782621 (the #10816 commits plus the two #10810 commits) on bare-metal Linux.

Setup: Ubuntu 26.04 x86-64 (i7-8700, not a VM), GCC 15.2, -DANYPS5_ENABLE_TSAN=ON (the option from #11179), RelWithDebInfo, TSAN_OPTIONS=suppressions=tools/tsan-suppressions.txt. I did not bypass the host CPU gate: this CPU passes it. Base: main c3ee5a425; the head merges cleanly. I merged it together with #10100 and #11125 for the run. Those two touch other files and other reports.

Baseline (main c3ee5a425): guest_pthread_stack reports the race in 10 of 10 runs. The read is scePthreadAttrGet (Attr.cpp:97, from RetainedStackWorker). The previous write is scePthreadDetach (Thread.cpp:1137, from main in GuestPthreadStack.cpp:219).

With this PR:

  • guest_pthread_stack and the new guest_pthread_detach_attr, 10 repeated runs: 0 reports, both pass.
  • guest_pthread_handle_after_exit and guest_pthread_join_detach pass with no report.
  • Full non-GPU suite: no report from Pthread/ code and no new report in any test.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@oneandonlydean
oneandonlydean merged commit 0ba2a2a into boykopovar:main Nov 2, 2026
10 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