Skip to content

fix(font): serialize shared FreeType face lifetime - #6282

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Ondra97:codex/fix-font-face-lifetime
Oct 18, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Ondra97:codex/fix-font-face-lifetime

Conversation

@Ondra97

@Ondra97 Ondra97 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

I reproduced a host font-memory leak on main d92a8dad390af1807cc91622b924252141816a5a. Eight workers use separate guest FontMemory/FontLibrary objects and valid immutable Arial data. All 4,000 public open/metrics/close sequences succeed with correct metrics, but only 3,994 host face finalizers run. At quiescent barriers, the shared FreeType driver lists eight faces instead of the nine still alive. The single-thread control finalizes all 500 faces.

State.cpp shares one host FT_Library. Its face creation runs outside stateMutex, while destruction under that mutex does not synchronize with creation. A lost driver-list node makes FT_Done_Face return without destroying the face. FreeType requires a common lock around creation and destruction on a shared library; the pinned header states the same rule at 3rdparty/freetype/include/freetype/freetype.h:547-550.

I protect cache face creation, replacement and destruction with one mutex. It is declared before the state map so cached faces can still be destroyed during static cleanup. The lock order is stateMutex then faceMutex; loading a face does not acquire stateMutex. Per-face glyph work remains outside this new lock. The shared source fixes the normal, module and native font builds.

The regression compiles the production State.cpp and observes actual FreeType face finalizers. It checks single-thread controls, concurrent open/close, replacement, and overlapping creation/destruction in separate processes, plus character mapping and glyph advance. It reuses the existing generated-font builder through FontFixture.hpp. The public API reproduction separately checks the built runtime library with a real host font; no font binary is added to the repository.

I checked the current open PR inventory, including all paths of 21 branches whose API file lists stop at 100, and compared the exact related implementations, tests and discussions. #6019 adds text-code stepping and tests; its GuestFont patch still applies after the fixture extraction. #5209 changes horizontal writing metrics and has a review about console text semantics. #6044 moves the unchanged GuestFont blob and changes test registration/labels. None changes the shared host library or face lifetime. The fixture include uses tests/FontFixture.hpp so its path also resolves after that test relocation.

Tested

Captured before/after measurements on the same PC (main d92a8dad3; counts describe the recorded runs, rather than an average):

Measurement Main Fix
Public API reproduction: actual face finalizers / 4,000 opened faces 3,994 / 4,000 4,000 / 4,000
Registered open/close regression: actual finalizers / 4,096 faces 4,009 / 4,096 4,096 / 4,096
Registered overlapping creation/destruction: actual finalizers / 4,096 faces 3,448 / 4,096 4,096 / 4,096
Registered replacement regression Heap corruption 0xc0000374 4,096 / 4,096 finalized
  • Windows 11 Pro 10.0.26300, supported WinLibs MinGW-w64 GCC 15.2.0, Release: full Windows build passed, including font variants and NID patching.
  • Hardware: AMD Ryzen 9 7900X, 32 GB RAM, NVIDIA GeForce RTX 4070 Ti SUPER.
  • Public API reproduction: main finalized 3,994/4,000 faces with six missing list entries; the fix finalized 4,000/4,000 with zero malformed-list observations and unchanged glyph metrics.
  • Negative control with only production State.cpp restored to main: all three registered regressions failed. The open/close case finalized 4,009/4,096 faces, creation/destruction finalized 3,448/4,096, and replacement exited with Windows heap corruption 0xc0000374. All single-thread controls finalized 128/128. The fixed source was restored and its SHA-256 checked before rebuilding.
  • Eleven relevant CTest entries cover patched libraries, existing guest font/system-font/writing-line tests, all three new lifetime tests, formatter/Windows exception tests, and AGC driver/graphics smoke tests. The final rebased head passed all eleven without skips. The lifetime-test timeout remains 30 seconds.
  • Twenty additional runs of each new test: all 60 passed, observing 253,440 face finalizers in total, with correct mapping/advance.
  • Test registration audit, conventions and whitespace check passed. The existing generated-font tests still pass after moving their builder to the shared fixture. GPU smoke tests require Vulkan and disable the shader disk cache.

Limits: no retail title, Linux runtime, console font/threading measurement or claim about console metrics. This fixes the measured host cache lifetime; existing guest-library semantics are unchanged. Full hosted Windows/Linux builds require collaborator dispatch under CONTRIBUTING.md; skipped jobs are not passes.

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: none
  • GPUs tested: NVIDIA GeForce RTX 4070 Ti SUPER, NVIDIA 617.14 (Windows driver 32.0.16.1714), Windows 11 Pro 10.0.26300 (additional renderer smoke tests)
  • AI-assisted: yes — Codex. I am responsible for the change.

@github-actions

github-actions Bot commented Oct 15, 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 6044 core/libs/prx/libSceFont/CMakeLists.txt
core/libs/tests/GuestFont.cpp

@Dextroid17

Copy link
Copy Markdown
Contributor

Linux run of this head, to go with the Windows measurements in the description.

Setup: head 7cdb99763 merged into main at 84b0d2f24 (clean merge; the head is 1,126 commits behind), BUILD_TESTING=ON, CMAKE_BUILD_TYPE=Release, Ninja, CachyOS (kernel 7.2.9), i9-11900K (16 threads), GCC 16.2.1. Tests ran under env -i with only HOME and PATH set. Each font_face_lifetime_tests mode (default, replace, churn) ran 10 times per arm, calling the test binary directly with a 60 s timeout.

  • Merged head: all three modes pass 10 of 10, with 4,096 / 4,096 finalizers in every 8-worker run. ctest -R font passes 6 of 6 in each of 3 runs. (An earlier set of 3 ctest runs reported 2 of 6 as Not Run because those two test binaries were not built yet; I built them and re-ran.)
  • Merged head, 12 copies at once (4 per mode), 5 rounds: 60 of 60 pass.
  • Only State.cpp from main, same tests: every mode fails 10 of 10.
    • Default: 3,880 to 3,945 of 4,096 finalizers.
    • churn: 831 to 3,985 of 4,096.
    • replace: 6 runs end in SIGSEGV, 3 in SIGABRT (malloc(): unaligned tcache chunk detected), and 1 finishes with 38 of 4,096 finalizers. This fits the heap corruption (0xc0000374) you saw in this case on Windows; the SIGSEGV runs print no heap message.
    • The single-worker controls finalize 128 of 128 in all 30 runs on both arms.

So the failures without the fix, and the passes with it, reproduce on Linux/glibc too, not only on Windows/MinGW. The hosted Linux and Windows CI jobs were skipped on this head.

By reading: in libSceFont, the new lock covers the only FT_New_Memory_Face call (LoadStateFace) and both FT_Done_Face calls (LoadStateFace and ~FontState). Where both locks are held, stateMutex comes first: ResetState and RemoveState destroy the old FontState (whose destructor takes faceMutex) while holding stateMutex, and LoadStateFace takes only faceMutex. I found no path that takes them in the opposite order.

Not checked: macOS, Clang, the public-API reproduction with a real host font, builds with sanitizers, and the head on its own base (only the merge into main was run).

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

AI-assisted: yes

@adriadam10 adriadam10 self-assigned this Oct 18, 2026
@adriadam10
adriadam10 merged commit 0bc3003 into boykopovar:main Oct 18, 2026
10 checks passed
@Ondra97
Ondra97 deleted the codex/fix-font-face-lifetime branch October 19, 2026 04:24
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