test: hold the recorder at each ordering point in test_recorder_sync's thread scenarios #2251

Merged

hckrman101 merged 1 commit from test/recorder-sync-deterministic into main

2026-11-11 12:30:01 +00:00

hckrman101 commented

2026-11-11 11:52:33 +00:00

(Migrated from github.com)

tests/test_recorder_sync.py sometimes fails on a busy CI runner for no reason in the code. It deliberately builds broken recorders and checks each one is caught. For the thread recorder, "caught" depended on timing: the fake driver takes 1 µs a call so the recorder thread falls behind, and the broken variant is only caught if it is still behind at the moment the producer should have waited. On a loaded runner the recorder sometimes catches up first, and the check reports a broken recorder as NOT CAUGHT. Two recent runs failed this way: 43006448114 ("the replay lock let go without waiting for the recorder") and 42987766874 ("no wait before vkQueueSubmit2"). Neither PR touched the recorder.

What changed

The thread scenarios that check ordering now hold the recorder thread for 50 ms just before each point where the producer must wait for it:

a recording's end;
a submit while another command buffer is still recording;
the replay lock being let go;
a pool reset.

This is the same hold the status-read scenario already used. A correct recorder simply waits out the hold. A broken one always reaches that point with commands still in flight, so it is caught every time, however loaded the runner is. The direct reference runs and the defer runs have no hold.

Test only: gpu/vk/vk_recorder.h and gpu/vk/vk_work.h are unchanged, and every existing check still runs.

How it was tested

tests/test_recorder_sync.py passes in full, and every broken variant (defer and thread) is reported caught.
Stress, on a busy 24-thread PC (load average 80–95 from other builds and games): I kept the built test binaries and ran the thread checks (--only thread) of the unbroken build and of each of the 11 deliberately broken thread variants 30 times. They ran as parallel jobs, each pinned to a single core, alongside the other builds. The unbroken build passed 30 of 30 times, and each broken variant was caught 30 of 30 times: 360 of 360, no flakes.
Before the change: the same 10 times with main's version of the test did not reproduce the flake on this PC (120 of 120). That fits a race that only lines up on CI's runners. Two such failures are linked above.
A thread-checks run takes the same time as before (about 26 s on one core). The holds add at most 50 ms at each of a dozen points.

`tests/test_recorder_sync.py` sometimes fails on a busy CI runner for no reason in the code. It deliberately builds broken recorders and checks each one is caught. For the thread recorder, "caught" depended on timing: the fake driver takes 1 µs a call so the recorder thread falls behind, and the broken variant is only caught if it is still behind at the moment the producer should have waited. On a loaded runner the recorder sometimes catches up first, and the check reports a broken recorder as `NOT CAUGHT`. Two recent runs failed this way: [43006448114](https://github.com/NotRexed/Hookshot/actions/runs/43006448114) ("the replay lock let go without waiting for the recorder") and [42987766874](https://github.com/NotRexed/Hookshot/actions/runs/42987766874) ("no wait before vkQueueSubmit2"). Neither PR touched the recorder. ## What changed The thread scenarios that check ordering now hold the recorder thread for 50 ms just before each point where the producer must wait for it: - a recording's end; - a submit while another command buffer is still recording; - the replay lock being let go; - a pool reset. This is the same hold the status-read scenario already used. A correct recorder simply waits out the hold. A broken one always reaches that point with commands still in flight, so it is caught every time, however loaded the runner is. The direct reference runs and the defer runs have no hold. Test only: `gpu/vk/vk_recorder.h` and `gpu/vk/vk_work.h` are unchanged, and every existing check still runs. ## How it was tested - `tests/test_recorder_sync.py` passes in full, and every broken variant (defer and thread) is reported caught. - **Stress, on a busy 24-thread PC** (load average 80–95 from other builds and games): I kept the built test binaries and ran the thread checks (`--only thread`) of the unbroken build and of each of the 11 deliberately broken thread variants 30 times. They ran as parallel jobs, each pinned to a single core, alongside the other builds. The unbroken build passed 30 of 30 times, and each broken variant was caught 30 of 30 times: 360 of 360, no flakes. - **Before the change:** the same 10 times with main's version of the test did not reproduce the flake on this PC (120 of 120). That fits a race that only lines up on CI's runners. Two such failures are linked above. - A thread-checks run takes the same time as before (about 26 s on one core). The holds add at most 50 ms at each of a dozen points.