Skip to content

fix(vcores): no core held while OpenFile builds the button prompts (#1929) - #2301

Merged
hckrman101 merged 1 commit into
NotRexed:mainfrom
mooki000:vcore-release-prompts
Nov 11, 2026
Merged

hckrman101 merged 1 commit into
NotRexed:mainfrom
mooki000:vcore-release-prompts

Conversation

@mooki000

Copy link
Copy Markdown
Contributor

When the button-prompt textures have to be written again (a first start, an update that changes them, another controller
face), OpenFile builds them inside host_path while keeping its virtual core: over 350 ms on Linux, and the threads
waiting for that core escaped and ran without one (#1929). This gives the core up for that build too, as OpenFile
already does for CreateFileA.

What changed

  • host/nn_fs.c OpenFile: the existing hos_vcore_away() / hos_vcore_back() pair now starts before host_path
    instead of just before CreateFileA. host_path reaches hos_mods_romfs -> hos_prompts_romfs, which builds the
    prompt font, textures or layouts on their first open (write_patched: a 244 MB file read and written when
    prompts/Common.key no longer matches). Threads that open the same file during the build wait for it there
    (while (state[which] == 1) Sleep(1)), and now do that without a core too. The early returns before CreateFileA
    take the core back before they return. There is still one release and one take per open, as before. Everything
    between host_path and CreateFileA is host code (redirects, the overlay check, a log line); no guest code runs
    without the core.
  • The core is not given up inside hos_prompts_romfs instead: host_path holds mount_lock (shared) there, and
    waiting for a core while holding it could stall a thread that holds a core and wants the lock (Mount, Unmount). That
    is the reason hos_cv_sleep lets go of its lock before it takes a core again.
  • tests/test_vcore_fs_open.py (new): compiles the actual OpenFile with stand-ins that record whether the thread holds
    its core in host_path and in CreateFileA, and checks every way out (opened, unknown mount, write to a mount that
    is not writable, HOS_NO_MOVIES, a blanked icon, not found), plus a thread that holds no core. It fails on main
    (host_path runs with the core held) and passes with this change, with g++ and clang++.

How it was found and tested

Linux (Arch), RTX 4090, NVIDIA 615.78.08, headless, fresh profile. The game was built with this patch on main 07b0359
(nothing since then touches nn_fs.c, mods.c or prompts.c). A local diagnostic build that logs every holder's host
stack at each escape found the holder here:
(game) > OpenFile > host_path > hos_mods_romfs > hos_prompts_romfs > build_textures > write_patched > fread / fwrite.
ARes Loading 0 had held core 5 for 357 ms by the last sample, asked to give it up, and four threads escaped within 42 ms
(ModuleSystemWorker5, GeneralFormatAsync_, ARes Loading 2, PreActorModelCompose).

The build runs only when prompts/Common.key no longer matches; otherwise the game reads the cached file and nothing
waits. So each run below removed Common.key first and ran 30 s with no input; the escape was set to 250 ms
(HOS_VCORE_ESCAPE_US/_EQ_US) so that waits over 250 ms show as escapes. Three runs each, interleaved, load 8-75
(other builds on the machine):

escapes per run of them while the prompt build held the core
main's code 1, 6, 1 0, 4, 0
this change 1, 1, 1 0, 0, 0

The one escape left in every run is the same in both: PreActorModelCompose waiting for sead::MainThread (in the
diagnostic runs, sead::MainThread was in game code at that moment). The build made other threads escape in one of the
three runs on main (in the other two nothing escaped behind it), and in none with the change. At the 2 s default nothing
escaped in either arm (three runs each), and the longest core wait in the first 5 s was about the same (main 877, 515,
482 ms; this change 480, 537, 462 ms): the build is not the longest wait at start-up.

tests/run_all.py on this branch's head (3ca9b79, on main 49f75dc): 363 passed, 2 failed, 10 skipped. The two
failures are the ones this machine has on every branch: test_updater_asset_digest.py (only the .NET 10 SDK here, the
fixture needs .NET 8) and test_wayland_libdecor_hint.py (clang 23's -Wunused-but-set-global under -Werror).

Not tested

Windows, AMD, the Steam Deck. On Windows the prompt files are built by the same code, so the same wait should happen
there, but it was not measured.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

…otRexed#1929)

The first open of a button-prompt file builds it inside host_path (prompts.c), 251-357 ms on Linux, and other
threads that open it meanwhile wait there for the build. OpenFile gave the virtual core up only around
CreateFileA, so the opener kept its core through the build, and four threads waiting for that core ran
without one (the escape) within 50 ms. The core now goes before host_path, with the same away/back pair.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CIB

CIB commented Nov 11, 2026 •

Copy link
Copy Markdown

Corroboration for the host-hold class, from our Linux runs.

We independently captured a vcore held across a host wait in event_signal (AcquireSRWLockExclusive(&e->lk) while vc_held) and fixed it with a try-lock that gives the core up on contention; the remaining escapes in our runs are non-polling host holders (OpusDecoder, the nn.npln worker/shared path). Your OpenFile prompt-build hold (251–357 ms) fits exactly that class: the existing away/back pair should cover host_path, and every early return must restore the core.

Two details worth checking in the final diff:

  • ensure no guest code or callback runs between the away and the back — a released core is fine, but a guest call there would run coreless with its stale TLS core number;
  • the prompt build is a first-start/update path, not the cue, so keep the pattern but do not expect it to change cue escapes.

We have a different local patch in nn_fs.c (handle lifetime), so there is no hunk conflict. The fixture test that compiles OpenFile and checks every return path is the right shape; adopt after review.


🤖 Generated with opencode using DeepSeek V4.1 Flash.

@mooki000

Copy link
Copy Markdown
Contributor Author

Thanks for reading it. Both checks, from the branch head (3ca9b79), host/nn_fs.c:

Nothing of the game runs between the away and the back. I read every call between line 332 (hos_vcore_away()) and each hos_vcore_back():

  • host_path (line 158): copies the path out of guest memory once, takes mount_lock shared, checks the path, then calls hos_mods_romfs (mods.c:182), which for a prompt file calls hos_prompts_romfs (prompts.c:500). That runs setup and choose_face (reads hookshot_options.ini, asks SDL for the first controller's name, Sleep(50) up to 30 times), then one of build, build_textures, build_layouts (file reads and writes), or Sleep(1) while another thread is building.
  • getenv, strlen / _stricmp / strstr on the path, hos_log, GetFileAttributesA, hos_ultrawide_redirect and hos_blank_cached_icon (both are return 0 for this game, mkw_glue.c:638-639), CreateFileA.

None of these is handed the guest context. I searched prompts.c and mods.c: no call into lifted code (a64_lookup, hos_call_guest, hos_ctx), no nn::os object, no event signal, no guest-memory access. The only guest memory touched in the window is the path string, and it is only read. The file handle is written to guest memory (line 381) after the core is back.

Every return takes the core back. There are five ways out after line 332 and five if (away) hos_vcore_back();: unknown mount (334), a write to a read-only mount (340), HOS_NO_MOVIES (348), a blanked icon (356), and the normal path right after CreateFileA (371; the not-found return and the success come after that line). tests/test_vcore_fs_open.py compiles the real function and checks each of them: the four early returns, not found, success, and a thread that held no core taking none.

One thing I left as it is on main, so that it is no surprise: host_path holds mount_lock shared through the build, so a thread that mounts or unmounts in those 251-357 ms (mount, line 96; Unmount, line 306) waits for that lock with its core held. I have not seen that in a run.

Agreed on the scope: it is a first-start path, not the cue. Our numbers say the same. In the issue's steps with the textures already built, no escape stood behind the build, and the change does not move the count: 6, 2, 3 escapes per run with it against 7, 8, 2 without (3 runs each, 250 ms). Only with the cache key deleted before the start does it show (30 s runs, 250 ms, 3 each): 1, 6, 1 without the change, 1, 1, 1 with it.

How those runs were made, so you can compare: HOS_VCORE_ESCAPE_US=250000 HOS_VCORE_ESCAPE_EQ_US=250000 in the environment, on builds that have #2127's 2 s default (880cf67 for main's code, 07b0359 plus this patch for the change). The branch is the same patch on 49f75dc (I compared the hunks). Linux, RTX 4090, NVIDIA 615.78.08, headless.

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

Looks right to me. I read OpenFile at 3ca9b79 end to end: there are five ways out after hos_vcore_away() (unknown mount, write to a read-only mount, HOS_NO_MOVIES, a blanked icon, and the one right after CreateFileA that both not-found and success go through), and each one takes the core back exactly once. host_path drops mount_lock before it returns, so the thread never waits for a core while holding the lock. Keeping the release out of hos_prompts_romfs for that reason is the right call.

Nothing between the away and the back calls into guest code. The only guest memory touched is the path string, which is only read (and host_path copies it once), and the handle is written to guest memory after the core is back. That answers both of CIB's checks. As you say, the Mount/Unmount thread that waits for mount_lock with its core held during a build was already there on main and isn't made worse here.

The test compiles the real function and covers every exit plus the no-core caller. It's picked up by run_all.py through the file name, with RUN_ALL_ARGS for the compiler.

On the siblings: no files overlap with #2302 (hos_thread.c/hos.h) or #2322 (nn_video.c), so they can land in any order. They also work together: with #2302, hos_vcore_away() returns 1 for a thread that escaped, and since every exit here pairs it with hos_vcore_back(), an escaped thread that opens a file takes a core again on its way out, which is what #2302 wants.

CI: only the title check has run. The Linux, Windows and source-regression workflows are waiting for a maintainer to approve the run, so they need to go green before merge.

@mooki000

Copy link
Copy Markdown
Contributor Author

Thanks for reading it end to end, and for checking it against #2302. Understood on CI: the three workflows wait for a maintainer's approval; I'll watch them and fix anything that comes up red.

@mooki000

Copy link
Copy Markdown
Contributor Author

Since this pull request and #2042 met on main (merged two minutes apart), tests/test_vcore_fs_open.py no longer compiles there, with g++ or clang++, so the game-free checks fail: #2042 changed OpenFile's HOS_NO_MOVIES read from getenv() to env_on(), and the test's fixture, written before that, has no env_on. The fix is in #2380 (the fixture gets gpu/vk/compat/hookshot_env.h with -include, as #2042 did for the other tests that cut code out of the sources), together with the same fix for #2322's test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants