Skip to content

fix(libSceNgs2): let render skip a grain while another thread holds the system lock - #7072

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
mrfelek:fix/ngs2-system-lock-try-render
Oct 17, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
mrfelek:fix/ngs2-system-lock-try-render

Conversation

@mrfelek

@mrfelek mrfelek commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

What

sceNgs2SystemLock took the library's process-wide Ngs2Mutex() and kept it until sceNgs2SystemUnlock. So while a title held the system lock, sceNgs2SystemRender and every other NGS2 call on any other thread waited for it.

On the console, render does not wait. prosper's notes, re-derived from the PS5 libSceNgs2.native module (found by @SandstromPL), say:

  • Lock takes a mutex in the system object and counts the hold.
  • Unlock releases it and returns 0 even without a Lock.
  • Render only try-locks it, and skips the grain and returns 0 while it is held.

Peace Walker (PPSA18216) deadlocks on this during its dlctex loads:

  • Its loader holds the system lock while it spins on a flag.
  • Its audio thread clears that flag only after sceNgs2SystemRender and eight sceNgs2VoiceGetStateFlags calls.
  • So on the console neither call can wait for the lock.

A core of a frozen run with render made non-blocking alone shows the audio thread blocked in sceNgs2VoiceGetStateFlags instead (details in #5268).

Changes:

  • Lock and unlock. sceNgs2SystemLock and sceNgs2SystemUnlock keep a per-system owner thread and depth behind their own mutex and condition variable, and no longer hold Ngs2Mutex() after they return.
    • Lock waits only while another thread holds the system lock.
    • Unlock releases one hold from any thread and returns 0 without a hold.
  • Render. sceNgs2SystemRender tries the system lock.
    • While another thread holds it, render clears one grain of each output (with the same buffer checks as a normal render), leaves voices and the render count unchanged, and returns 0.
    • Otherwise it holds the lock while it renders, so a sceNgs2SystemLock from another thread waits for the grain.
  • Other calls. Every other call takes Ngs2Mutex() only for its own work, so none of them waits for the system lock.
  • Unchanged. sceNgs2RackLock is unchanged.
  • Debt note. The debt entry says which parts come from prosper's notes and that the console's voice and rack calls are not known to take the lock.

Tested

Linux (Ubuntu 26.10), GCC 15.3, Release, Ryzen 9 5950X.

libSceNgs2 Runs Kept presenting after the dlctex loads
main's lock 2 0 (frozen, deadlock read with gdb)
render made non-blocking only 1 0 (frozen in sceNgs2VoiceGetStateFlags)
this PR 4 4 (24 [present] lines in the 240 s after the last load each, then it opens its music file; no NGS2 errors)

The same freeze happens on Windows 11 with main's lock (3 of 4 runs, RTX A2000 Laptop). This PR was not run on Windows.

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: PPSA18216 METAL GEAR SOLID: Peace Walker - Master Collection Version (PPSA18216) #5268
  • GPUs tested: n/a
  • AI-assisted: yes

AI-assisted: this PR was written with the help of Claude Code and checked against the runs by us.

…he system lock

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BywPXoZACVi8ALV4JKvuPZ
@github-actions

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 1384 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
core/libs/prx/libSceNgs2.native/src/System.cpp
PR 5797 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/Render.cpp
docs/dev/debt/libSceNgs2.native.md
PR 6447 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
docs/dev/debt/libSceNgs2.native.md
PR 6594 docs/dev/debt/libSceNgs2.native.md
PR 6680 core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp
core/libs/prx/libSceNgs2.native/src/System.cpp
docs/dev/debt/libSceNgs2.native.md

@SandstromPL

Copy link
Copy Markdown
Contributor

@mrfelek thanks for picking this up, and for the core dump that showed the sceNgs2VoiceGetStateFlags block.

I built and tested the head (40f4506) on Ubuntu 24.04, GCC 13.3, Release:

  • All 11 ngs2_* tests pass, including ngs2_core_render.
  • 50 repeats of each render test, plus 1520 runs under heavy parallel load (down to one pinned CPU): 0 failures.
  • ThreadSanitizer, 6 runs: no warnings.
  • With main's System.cpp, both render tests abort at line 907, as you said.
  • Not run: Windows, or any game.

The Peace Walker path looks right to me. A few points:

  1. A stray Unlock during a render can drop another thread's hold. Render adds its hold to the same owner/depth as the game, and both sceNgs2SystemUnlock and Render's Release decrement without checking the owner. A small test program reproduces it 10 times out of 10:

    • R is rendering.
    • U calls Unlock without a Lock.
    • L's Lock succeeds mid-grain.
    • R's Release then drops L's hold, so other threads render and lock while L thinks it holds the lock.

    Main doesn't do this. A fix: a separate renderer field in SystemLockState. Render sets it, Release clears it and calls notify_all, and Lock also waits until it is empty (or is the calling thread). Your TestLock passes unchanged with that.

  2. "Unlock releases from any thread, as on the console" was my overstatement in METAL GEAR SOLID: Peace Walker - Master Collection Version (PPSA18216) #5268, sorry. prosper's notes only say Unlock ignores the unlock result. Their "need not be the one that locked" line explains prosper's own approximation, not the native module. On a non-owner unlock, FreeBSD libthr returns EPERM for every mutex type, and so does our own libkernel. So the console most likely answers 0 and keeps the lock. Two options:

    • Owner-only Unlock: if (owner == self) ReleaseSystemLock(...). Lines 929-934 would then expect silence until the owner unlocks.
    • Or keep it, and call it an emulator choice in the debt line.

    Owner-only shouldn't affect Peace Walker. Your render-drop runs still used main's Lock/Unlock, where an Unlock from another thread would block forever, and 2 of 3 kept presenting. Can you confirm Peace Walker locks and unlocks on the same thread?

  3. One new deadlock. sceNgs2RackLock still holds Ngs2Mutex after it returns. Suppose a thread holds it, or is inside an NGS2 callback, and calls sceNgs2SystemLock while another thread holds the system lock. Then the holder's next NGS2 call blocks, and both threads hang. Main completes this; the PR deadlocks 10 times out of 10. It's unlikely in games, so one sentence in the debt entry should be enough.

  4. Debt line. Besides point 2, "follow prosper's notes" also covers things the notes don't give:

    • nested holds, and rendering on the holding thread (the notes say recursion "is not pinned");
    • clearing one grain while keeping the render count.

    Could the line be split into what the notes say and what this library assumes?

  5. Optional tests (happy to share snippets):

    • a skip with a playing voice and two outputs ("voices unchanged" and "each output" aren't checked today);
    • Destroy waking a thread that is waiting in Lock;
    • a bounded poll instead of blocked.join(), so a regression fails with a line number instead of hanging.

Last thing: your Peace Walker runs were on a port to 8856f18, where the silent grain cleared whole buffers. One run on this exact head, ideally with points 1-2 in, would be great.

AI-assisted: yes (Claude Code)

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