fix(vk): seed_keys is filled only for RX_VK_SEED_CHECK, its one reader #393

Open

madebyjordan wants to merge 3 commits from madebyjordan/Longshot:fix/seed-keys-race into main

pull from: madebyjordan/Longshot:fix/seed-keys-race

merge into: MagicBeanDev:main

MagicBeanDev:main

Conversation 1 Commits 3 Files changed 4 +53 -1

madebyjordan commented 2026-11-14 08:07:53 +00:00

Contributor

• Copy link
• Copy Markdown

Why

When the shaders' preparation runs during play (RX_VK_PREPARE_BACKGROUND), it can fill a hash set while the renderer's thread is reading it. That's a data race that can walk freed memory and crash. The set only serves a debugging switch, so now it's filled only when that switch is on.

What changed

• vk: in seed_open() (gpu/vk/vk_shader.cpp), every seed stage's key goes into seed_keys only when RX_VK_SEED_CHECK=1 (read once, seed_check).
  • seed_keys (a std::unordered_set) has one reader: miss_note, which marks the replay's program misses (the first 1,500 in play) with the stages the seed holds.
  • On main, a seed opened in play by prepare_background inserted every key while those reads ran. A rehash under a reader can walk freed buckets.
  • Without RX_VK_SEED_CHECK the set now stays empty, and miss_note reads an empty set that nothing writes.
• tests: tests/test_seed_keys_race.py (new) reads seed_open() and checks that every seed_keys.insert is under seed_check, and that seed_check is RX_VK_SEED_CHECK read once. It fails on main.
• docs: docs/development.md, an RX_VK_SEED_CHECK row; it wasn't in the switch table, only in docs/BAKE.md.
• tests: tests/test_seed_reader.py, which compiles the real seed_open() against stand-ins, gets one for env_on. Its 20,000 mutations are unchanged.

Flags and settings

Flag	What it does	Default now	Recommendation	Helps on	Doesn't help on	Untested on
RX_VK_SEED_CHECK=1 (existing)	Marks the replay's program misses with the seed's stages (miss_note's +); now also the only time seed_keys is filled	off	Debugging only, as before. With it on, the in-play fill can still race the reads: it's a developer switch	n/a	n/a	n/a

Measurements

A fix of a timing-dependent race, so the evidence for the race is the test. The game runs show the fix changes nothing else.

main: cf605c85 (2026-11-14)
this PR: fix/seed-keys-race @ a4fa2ab0
Rebased since onto main 0caf0618 (now e867b434): the numbers still hold. #357 and #358, merged since, change the lift and the button prompts, not the renderer.
Hardware: Ryzen 5 5600, 16 GB, AMD Radeon RX 6700 XT, AMD driver 26.10.1, Windows 11
Scene: the path where the seed is first opened in play. An update start (another translator's prepare record kept, caches removed), RX_VK_PREPARE_BACKGROUND=1 RX_VK_NO_STATE_LIST=1, so prepare_background opens the seed 20 s into play while play misses programs. Then HOS_SKIP_OPENING=1, New Game into Link's house and Kokiri Forest, headless, 480 s, a cold driver cache each run. With the state list on, the seed opens before the title and the race can't happen.
Method: longshot_vk.dll built from each commit (RX_VK_ONLY=1 RX_RELEASE=1, llvm-mingw clang 23) into the same game folder, main and this PR alternated, 2 runs each. Read from the game log (miss_note lines, crash lines) and RX_VK_FRAME_LOG.
How to reproduce: the scene above, with each build's longshot_vk.dll; count the miss_note lines with a + after the in-play seed opens.

	main @ cf605c85	this PR @ a4fa2ab0
full 480 s, no crash	2 of 2	2 of 2
miss_note lines after the seed opened in play	130, 131	179, 133
of them with a + (the set read while it was filled)	130, 131 (all)	0, 0
longest frame (the cold load's compile stall, the same on both)	129.9 s, 126.1 s	101.0 s, 123.9 s
fps after the load (median frame, from 30 s after it)	48.9, 50.5	49.9, 48.5

The lines name the same programs: main's two runs share 140 of 144/145, and the second PR run names 2 programs neither main run did. The first PR run walked further (a shorter stall, the same timed inputs), hence its 58 more lines. The stall varies this much on main too. The race didn't visibly fire in these four runs; it depends on timing.

	main @ 0caf0618	this PR @ e867b434
tests/test_seed_keys_race.py	fails	passes
tests/run_all.py --jobs 4	485 passed, 6 failed, 70 skipped	486 passed, the same 6 failed, 70 skipped (the 6 fail on main on this PC: test_module_ids, test_oot_state_tap, test_pad_layout, test_portable_builder_home, test_portable_builder_startup, test_tested_configurations)

Testing

Tested:

• The runs above (Windows, AMD).
• tests/test_seed_keys_race.py.
• tests/test_seed_reader.py with clang 23.
• python -I -B tests/run_all.py --jobs 4 on main and this PR.

Not tested:

• Linux: the renderer wasn't built for it here. The change is a static const bool and an if, in code Linux already compiles.
• NVIDIA and Intel: the change is the same on every driver, and is only the set's fill.

Needs other testers:

• Linux (any GPU): the renderer builds, and a start with RX_VK_PREPARE_BACKGROUND=1 plays into Kokiri without a crash (T-5QMM).

Screenshots

Nothing on screen.

Risk and rollback

• gpu/vk/vk_shader.cpp is part of the translator's stamp, so every player's programs are translated once more after this lands, as with any release that touches the translator.
• With RX_VK_SEED_CHECK=1 the set is filled as before, and so is the race; that's a developer switch.
• Reverting the commit gives main's behaviour.

Issues

Found while measuring the in-play shader preparation (#362's shader work).

Checklist

For everyone

• One topic.
• The title is a Conventional Commit (type(scope): description).
• Rebased on current main.
• python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
• Tested in the game, or what wasn't is named under Testing.
• Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
• Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
• No game files, dumps, build output or generated files.
• third_party/ untouched (a new folder with a PROVENANCE.md instead).
• The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

• AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
• A game-free test added for the fix, failing on main and passing here.
• A check under tests/ that reads changed source updated in this PR, not weakened.
• Flags noted, and the switch reference updated (docs/development.md, the README).
• Docs updated.

## Why When the shaders' preparation runs during play (`RX_VK_PREPARE_BACKGROUND`), it can fill a hash set while the renderer's thread is reading it. That's a data race that can walk freed memory and crash. The set only serves a debugging switch, so now it's filled only when that switch is on. ## What changed - vk: in `seed_open()` (`gpu/vk/vk_shader.cpp`), every seed stage's key goes into `seed_keys` only when `RX_VK_SEED_CHECK=1` (read once, `seed_check`). - `seed_keys` (a `std::unordered_set`) has one reader: `miss_note`, which marks the replay's program misses (the first 1,500 in play) with the stages the seed holds. - On main, a seed opened in play by `prepare_background` inserted every key while those reads ran. A rehash under a reader can walk freed buckets. - Without `RX_VK_SEED_CHECK` the set now stays empty, and `miss_note` reads an empty set that nothing writes. - tests: `tests/test_seed_keys_race.py` (new) reads `seed_open()` and checks that every `seed_keys.insert` is under `seed_check`, and that `seed_check` is `RX_VK_SEED_CHECK` read once. It fails on main. - docs: `docs/development.md`, an `RX_VK_SEED_CHECK` row; it wasn't in the switch table, only in `docs/BAKE.md`. - tests: `tests/test_seed_reader.py`, which compiles the real `seed_open()` against stand-ins, gets one for `env_on`. Its 20,000 mutations are unchanged. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_SEED_CHECK=1` (existing) | Marks the replay's program misses with the seed's stages (`miss_note`'s `+`); now also the only time `seed_keys` is filled | off | Debugging only, as before. With it on, the in-play fill can still race the reads: it's a developer switch | n/a | n/a | n/a | ## Measurements A fix of a timing-dependent race, so the evidence for the race is the test. The game runs show the fix changes nothing else. main: cf605c85 (2026-11-14) this PR: fix/seed-keys-race @ a4fa2ab0 Rebased since onto main 0caf0618 (now e867b434): the numbers still hold. #357 and #358, merged since, change the lift and the button prompts, not the renderer. Hardware: Ryzen 5 5600, 16 GB, AMD Radeon RX 6700 XT, AMD driver 26.10.1, Windows 11 Scene: the path where the seed is first opened in play. An update start (another translator's prepare record kept, caches removed), `RX_VK_PREPARE_BACKGROUND=1 RX_VK_NO_STATE_LIST=1`, so `prepare_background` opens the seed 20 s into play while play misses programs. Then `HOS_SKIP_OPENING=1`, New Game into Link's house and Kokiri Forest, headless, 480 s, a cold driver cache each run. With the state list on, the seed opens before the title and the race can't happen. Method: `longshot_vk.dll` built from each commit (`RX_VK_ONLY=1 RX_RELEASE=1`, llvm-mingw clang 23) into the same game folder, main and this PR alternated, 2 runs each. Read from the game log (`miss_note` lines, crash lines) and `RX_VK_FRAME_LOG`. How to reproduce: the scene above, with each build's `longshot_vk.dll`; count the `miss_note` lines with a `+` after the in-play seed opens. | | main @ cf605c85 | this PR @ a4fa2ab0 | |---|---|---| | full 480 s, no crash | 2 of 2 | 2 of 2 | | `miss_note` lines after the seed opened in play | 130, 131 | 179, 133 | | of them with a `+` (the set read while it was filled) | 130, 131 (all) | 0, 0 | | longest frame (the cold load's compile stall, the same on both) | 129.9 s, 126.1 s | 101.0 s, 123.9 s | | fps after the load (median frame, from 30 s after it) | 48.9, 50.5 | 49.9, 48.5 | The lines name the same programs: main's two runs share 140 of 144/145, and the second PR run names 2 programs neither main run did. The first PR run walked further (a shorter stall, the same timed inputs), hence its 58 more lines. The stall varies this much on main too. The race didn't visibly fire in these four runs; it depends on timing. | | main @ 0caf0618 | this PR @ e867b434 | |---|---|---| | `tests/test_seed_keys_race.py` | fails | passes | | `tests/run_all.py --jobs 4` | 485 passed, 6 failed, 70 skipped | 486 passed, the same 6 failed, 70 skipped (the 6 fail on main on this PC: test_module_ids, test_oot_state_tap, test_pad_layout, test_portable_builder_home, test_portable_builder_startup, test_tested_configurations) | ## Testing **Tested:** - The runs above (Windows, AMD). - `tests/test_seed_keys_race.py`. - `tests/test_seed_reader.py` with clang 23. - `python -I -B tests/run_all.py --jobs 4` on main and this PR. **Not tested:** - Linux: the renderer wasn't built for it here. The change is a `static const bool` and an `if`, in code Linux already compiles. - NVIDIA and Intel: the change is the same on every driver, and is only the set's fill. **Needs other testers:** - Linux (any GPU): the renderer builds, and a start with `RX_VK_PREPARE_BACKGROUND=1` plays into Kokiri without a crash (T-5QMM). ## Screenshots Nothing on screen. ## Risk and rollback - `gpu/vk/vk_shader.cpp` is part of the translator's stamp, so every player's programs are translated once more after this lands, as with any release that touches the translator. - With `RX_VK_SEED_CHECK=1` the set is filled as before, and so is the race; that's a developer switch. - Reverting the commit gives main's behaviour. ## Issues Found while measuring the in-play shader preparation (#362's shader work). ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] Docs updated.

madebyjordan added 2 commits 2026-11-14 08:07:53 +00:00

fix(vk): seed_keys is filled only for RX_VK_SEED_CHECK, its one reader ... 6032d54265

seed_open() inserted every seed stage's key into seed_keys; miss_note reads it on the replay's program picks (the first
1500 misses in play). A seed opened in play (prepare_background) filled it while those reads ran: a std::unordered_set
rehashed under a reader, which can walk freed buckets. Now filled only when RX_VK_SEED_CHECK=1 asks for the marks.
vk_shader.cpp is part of the translator's stamp: this change translates the programs again once, as any release does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): seed_open fills seed_keys only for RX_VK_SEED_CHECK ... e867b43417

tests/test_seed_keys_race.py reads seed_open() in gpu/vk/vk_shader.cpp: every seed_keys.insert is under seed_check, and
seed_check is RX_VK_SEED_CHECK read once. It fails on main, where every seed stage's key went in while miss_note read
the set in play. tests/test_seed_reader.py, which compiles the real seed_open against stand-ins, gets one for env_on.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

madebyjordan changed title from WIP: fix(vk): seed_keys is filled only for RX_VK_SEED_CHECK, its one reader to fix(vk): seed_keys is filled only for RX_VK_SEED_CHECK, its one reader 2026-11-14 08:08:10 +00:00

madebyjordan added 1 commit 2026-11-14 08:08:38 +00:00

docs: RX_VK_SEED_CHECK in the switch table ... 74e7350f26

Co-Authored-By: Claude Opus 5.5 <[email protected]>

madebyjordan commented 2026-11-14 08:08:48 +00:00

Author

Contributor

• Copy link
• Copy Markdown

Pushed 74e7350f: RX_VK_SEED_CHECK gets a row in docs/development.md's switch table (it was only in docs/BAKE.md), since this changes what the switch does. Docs only; no code change.

Pushed 74e7350f: `RX_VK_SEED_CHECK` gets a row in docs/development.md's switch table (it was only in docs/BAKE.md), since this changes what the switch does. Docs only; no code change.

mooki added the

kind

bug

priority

high

labels 2026-11-14 08:27:42 +00:00

mooki added the

area/renderer

label 2026-11-14 16:27:47 +00:00

Some checks are pending

Hide checks

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.

Some workflows are waiting to be reviewed.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'fix/seed-keys-race:madebyjordan-fix/seed-keys-race'

git switch 'madebyjordan-fix/seed-keys-race'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area/accessibility

Barriers for people with disabilities

  

area/audio

Sound and music

  

area/input

Controllers, keyboard, mouse

  

area/install

Installing, updating, building from your game

  

area/menu

In-game menu and settings

  

area/renderer

Vulkan renderer, shaders, effects

  

area/saves

Save files

  

area/upscaling

DLSS, FSR, frame generation

  

good first issue

A good start for a new contributor

  

gpu/amd

AMD GPUs

  

gpu/igpu

Integrated GPUs

  

gpu/intel

Intel GPUs

  

gpu/nvidia

NVIDIA GPUs

  

help wanted

Extra hands or knowledge wanted

  

kind

bug

Something doesn't work as it should

  

kind

chore

Build, CI, dependencies, tidying

  

kind

docs

Documentation

  

kind

feature

A new feature or an improvement

  

kind

performance

Frame rate, frame time, loading, memory

  

kind

question

A question rather than a change

  

needs measurements

Waiting for before/after measurements

  

platform/linux

Linux only

  

platform/steam-deck

Steam Deck

  

platform/windows

Windows only

  

preview

Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  

priority

critical

Crashes, data loss, or blocks most players

  

priority

high

Important: next in line

  

priority

low

Nice to have

  

status

blocked

Waits on something else first

  

status

confirmed

Reproduced or agreed on

  

status

duplicate

Already reported or proposed

  

status

invalid

Not a problem in this project

  

status

needs-info

Waiting for more information from the reporter

  

status

wontfix

Will not be worked on

  

testing wanted

Wanted: a test on other hardware (posts a test request in #test-requests)

No labels

area/accessibility

area/audio

area/input

area/install

area/menu

area/renderer

area/saves

area/upscaling

good first issue

gpu/amd

gpu/igpu

gpu/intel

gpu/nvidia

help wanted

kind

bug

kind

chore

kind

docs

kind

feature

kind

performance

kind

question

needs measurements

platform/linux

platform/steam-deck

platform/windows

preview

priority

critical

priority

high

priority

low

status

blocked

status

confirmed

status

duplicate

status

invalid

status

needs-info

status

wontfix

testing wanted

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

MagicBeanDev/Longshot!393

No description provided.