Skip to content

fix(libkernel): keep Unity GC delivery out of MinGW TLS waits - #1562

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
hamb3r:codex/fix-unity-suspendpoint-fls
Oct 10, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
hamb3r:codex/fix-unity-suspendpoint-fls

Conversation

@hamb3r

@hamb3r hamb3r commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Unity's stop-the-world collector can raise SIGUSR1 while a worker is inside a host mutex wait. The previous Windows path could remain blocked in MinGW's TLS implementation: the signal handler re-entered guest thread state, pthread TSD or TimedWait state while the interrupted code still held an internal MinGW TLS lock. That left the collector waiting for a thread that could not run its handler and produced repeated sce::Agc::suspendPoint messages associated with the TRC R5089 watchdog.

This change keeps the signal-visible state out of MinGW TLS on Windows:

  • guest current-thread state and pthread TSD use native Windows FLS;
  • TimedWait state and per-thread timers use the same FLS-backed storage;
  • a contended scePthreadMutexLock uses short alertable waits through try_lock, allowing the GC delivery path to run while the mutex owner is still held;
  • blocking read/write rwlock waits use the same interruptible polling path;
  • the existing Linux paths retain thread_local storage and the existing condition-variable implementation.

The default mutex type change (ErrorCheck to Normal) was removed after it caused a guest_kernel_errors regression. It belongs in a separate change.

A permanent Vulkan WaitIdle was intentionally not added. Forcing Vulkan idle did not remove the loading stall, so suspendPoint is fixed at the thread-synchronization layer.

This PR complements #1476, which fixes nested TimedWait waiter ownership. The measured PPSA28997 freeze requires both parts.

Tests

Added guest_mutex_raise_exception, a Windows regression test that:

  • holds a mutex from the main thread;
  • blocks a worker in scePthreadMutexLock;
  • raises SIGUSR1 on the blocked worker;
  • verifies handler delivery;
  • releases the mutex and verifies that the worker acquires, releases and joins successfully.

Tested

God of War: Sons of Sparta (PPSA28997), Windows 10, NVIDIA GeForce RTX 3080.

The game reached the normal menu with the shader cache populated. The repeated Forcing call to sce::Agc::suspendPoint to avoid TRC R5089 breach loop did not recur during the verified run.

Targeted Windows tests: guest_raise_exception, guest_mutex_raise_exception and guest_pthread_key. CI runs the complete platform test suites.

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 in TechnicalDebt
  • 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: fix(libkernel): give each nested TimedWait its own waiter on Windows #1476 for nested TimedWait waiter ownership
  • AI-assisted: yes

@oneandonlydean @adriadam10

This PR addresses the Unity GC/suspendPoint stall by removing MinGW TLS re-entry from the Windows signal path and making contended mutex and rwlock waits interruptible. The Vulkan path is deliberately unchanged because WaitIdle did not resolve the stall. The fix was verified on Windows 10 with an NVIDIA RTX 3080 while running God of War: Sons of Sparta (PPSA28997).

@merwie

merwie commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Tested on Windows 11 Pro 26200, WinLibs GCC 15.2.0 (r7), Release, at f6127d3, AMD GPU.

Build and tests. Configures and builds. ctest -j8 -E "agc_driver|graphics": 444/445. guest_kernel_errors fails:

terminate called after throwing an instance of 'std::runtime_error'
  what():  Mutex is already owned by the current thread

It passes on main. The cause is the default type change in PthreadMutexPrivate (ErrorCheck → Normal): a relock of a mutex created without attributes now throws instead of returning EDEADLK. That change doesn't seem needed for the GC fix. Could it be dropped or moved to its own PR?

Same title (PPSA28997), measured as "frames still presented 40 s after start" over 6 runs each:

Build Runs past the intro
main + #1534 about 1 in 5
+ #1562 1 of 6
+ #1562 + #1476 6 of 6
+ #1476 + an equivalent interruptible scePthreadMutexLock (instead of #1562) 6 of 6

So on this machine the freeze needs both parts: the interruptible mutex wait (here or in an equivalent form) and #1476. With APS5_HANG_DUMP_SECS and a trace of raises and deliveries, the frozen runs show every raised thread inside its handler, and the collector polling with sceKernelUsleep for an acknowledgement that never comes. That is #1476's lost wakeup: the handler's sceKernelSyncOnAddressWait reused the waiter of the interrupted wait.

One more gap of the same kind: scePthreadRwlockRdlock and scePthreadRwlockWrlock still block in lock_shared() / lock(), so a raise cannot reach a thread waiting there either.

Tested with AI assistance (Claude Code).

@hamb3r

hamb3r commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

@merwie thank you for the testing and measurements.

I addressed your findings:

  • removed the default mutex type change (ErrorCheck to Normal), which caused the guest_kernel_errors regression;
  • made scePthreadRwlockRdlock and scePthreadRwlockWrlock waits interruptible;
  • added guest_mutex_raise_exception to verify GC signal delivery while a worker is blocked in scePthreadMutexLock;
  • updated the PR description and explicitly listed fix(libkernel): give each nested TimedWait its own waiter on Windows #1476 as a dependency, since nested TimedWait waiter ownership is fixed there.

The PPSA28997 measurements and the conclusion that both parts are required are recorded in the PR description.

@popcoder565

Copy link
Copy Markdown
Contributor

Windows measurements of your current head, since I'd compared an older one in #1421's description (I've updated it there). Setup: Windows 10 (19045), GCC 15.2.0 (WinLibs r7), Release, de12706a merged onto main 6cc0503 with #2093, which main needs to build right now.

  • guest_raise_exception passes 10/10 with main's test file. It passes 30/30 with fix(libkernel): stop losing and deadlocking sceKernelRaiseException deliveries on Windows #1421's busy, continuing and selfing phases. The full suite passes 3 of 3 runs under -j10. The hangs I'd seen at 5bda2a7c are gone.
  • One case still hangs: a handler that uses a thread_local from another module while the target was stopped inside winpthreads.
    • In my probe, a worker loops on libc's strerror, whose buffer is a MinGW thread_local, and the handler calls strerror too.
    • With this PR, the handler hung in all 5 runs, in rounds 2–70 of 500. main hangs the same way.
    • With fix(libkernel): stop losing and deadlocking sceKernelRaiseException deliveries on Windows #1421's retry, which keeps a raise from stopping a thread inside libwinpthread-1.dll, all 500 rounds finished in 5 of 5 runs.
    • Moving libkernel's state to FLS fixes libkernel's own calls, but libc and the other prx modules still use emutls.

🤖 Addressed by Claude Code

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please cut this down to the interruptible lock waits that PPSA28997 needs. Four things before it can go in:

@hamb3r
hamb3r force-pushed the codex/fix-unity-suspendpoint-fls branch from de12706 to 0761021 Compare October 9, 2026 10:05
@hamb3r

hamb3r commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@oneandonlydean Done. Reverted Exception.cpp and ools/check_test_registration.py to main. Moved the new test inside BUILD_TESTING in CMakeLists. Limited the 1ms ry_lock polling in Mutex/Rwlock strictly to Windows.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

This conflicts with main after today's merges; could you rebase? It's otherwise ready.

@hamb3r
hamb3r force-pushed the codex/fix-unity-suspendpoint-fls branch from 0761021 to 14f5572 Compare October 10, 2026 00:07
@hamb3r

hamb3r commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

@oneandonlydean Rebased onto current main, resolved the conflict, and verified conventions check locally. Ready for merge.

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

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#1739 core/libs/prx/libkernel/Pthread/src/Thread.cpp L2, L46-47, L441-442, L472
#2051 core/libs/prx/libkernel/Pthread/src/Mutex.cpp L164
core/libs/prx/libkernel/Pthread/src/Rwlock.cpp L44-45, L64-65
#2545 core/libs/prx/libkernel/Pthread/include/WindowsThreadLocal.hpp
core/libs/tests/GuestMutexRaiseException.cpp

oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 10, 2026
oneandonlydean added a commit to oneandonlydean/AnyPS5 that referenced this pull request Oct 10, 2026
@oneandonlydean
oneandonlydean merged commit 80d539a into boykopovar:main Oct 10, 2026
8 checks passed
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…dpoint-fls

fix(libkernel): keep Unity GC delivery out of MinGW TLS waits
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