MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix(vk): keep programs the driver faulted on out of the driver (#996) #1823

Merged
getzabelz merged 2 commits from fix/driver-fault-quarantine into main 2026-11-11 16:36:06 +00:00
Conversation 10 Commits 2 Files changed 7 +410 -9
getzabelz commented 2026-11-10 17:38:27 +00:00 (Migrated from github.com)

Fixes #1740
Fixes #1734
Related to #996: the freeze on closing the pause menu or map, with the music still playing.

The freeze

On AMD Windows (driver 0x800161), the game freezes when closing the pause menu or the map in Kokiri Forest and Hyrule Market, with the music still playing. There is no TDR and no device lost.

I took a full process dump of the freeze with Task Manager (RX 6800 XT, v2.0.13, closing the pause menu in Hyrule Market). I then read the thread stacks with the symbols in hookshot.exe / hookshot_vk.dll:

Thread Where it was
sead::MainThread event_wait (waiting for the frame)
Presentation Thread mkwvk::draw → targets → jobs_wait, for program 6eaa646d07ea4e43's pipeline
mkw_vk worker graphics_pipeline job → build_graphics → guarded_graphics → amdvlk64.dll, SleepConditionVariableCS, program 6eaa646d07ea4e43
precompile thread precompile_run → build_graphics → amdvlk64.dll, SleepConditionVariableCS, program 6eaa646d07ea4e43
precompile thread same, program 5fa56485a6b07619

I found the program keys by following the Program& on each stack.

Both programs are among the ones every AMD log reports at start as graphics pipeline for 5fa56485a6b07619 / 6eaa646d07ea4e43 / 4e5ce183a33b0855 not made (-13). That -13 is the __except in guarded_graphics, which catches an access violation inside AMD's compiler. Those two stuck precompile threads are also the precompile: N of N+2 done, nothing finished for 30 s of every AMD start.

What happens: the caught fault leaves that program's compile unfinished inside the driver. Every later vkCreateGraphicsPipelines of the same program, with any state, waits inside the driver for a condition variable that is never signalled. The game runs until it needs a new pipeline of one of those programs (closing the pause menu, some areas), and then the frame never finishes.

The fix

gpu/vk/driver_faults.h (new, Vulkan-free) and build_graphics:

  • Quarantine: a program the driver faulted on is not asked of it again for the rest of the run. Its draws are left out, as they already were since its pipeline was never made.
  • One build of a program in the driver at a time, on AMD: a second build of the same program waits in Hookshot's gate, outside the driver. If the first one faulted, the second is refused instead of being caught inside the driver behind it. This makes the first run safe too. A cold probe (FAIL_ON_PIPELINE_COMPILE_REQUIRED) is never made to wait: a build of its program under way answers it as a compile that is required. This gate is on for AMD (vendor 0x1002) by default, where the hang was seen. Other drivers build a program's state variants side by side, as before, and only the quarantine applies to them.
  • Across runs: the faults are written to hookshot_driver_faults.txt beside the executable, as <vendor> <driver version> <key> lines. On later runs with the same vendor and driver those programs are never asked of the driver, the precompile included. A driver update tries them again. If the file cannot be written (a read-only folder), the log says so once and the quarantine lasts for that run.
  • Telling a fault apart: the graphics guards (Windows SEH and Linux signal) now answer a fault with -77, as the compute guards already did. A driver's own VK_ERROR_UNKNOWN is no longer taken for a fault. The Windows guards keep #1766's log line with the exception code.
  • No stray handle after a fault (suggested by @Arkex-ai): on the caught-fault branch only, all four guards set *pl = VK_NULL_HANDLE. Whatever the driver wrote before it faulted is never published or destroyed; for example, fallback_compute_pipelines() destroys every non-null output on an error.
  • Switch: RX_VK_DRIVER_FAULTS=0 turns all of this off; =2 turns the gate on for every driver.

With #1747: its AMD retry without the optimizer followed VK_ERROR_UNKNOWN, which until now also meant a fault. It still runs after a fault, now inside the gate. The program is quarantined only if the retry faults too.

With #1772: the gate wraps graphics_with. That can reach the driver up to three times through the play caches, all inside the gate. create_cached returns the guard's result unchanged, so a fault still reaches the quarantine.

Results (RX 6800 XT, driver 0x800161, Windows 11)

The builder's precompile runs on the player's GPU, so the build log shows the effect directly:

Driver faults Precompile threads stuck forever Precompile
Before 3 2 32.5 s, 6625 of 6627 done, nothing finished for 30 s
First run with the fix 3, quarantined; the second builds of 5fa56485 and 6eaa646d refused outside the driver 0 4.2 s, complete
Next runs 0 (3 programs skipped from the file) 0 2.3 s, complete

In game, before the fix, it froze on closing the pause menu in Hyrule Market, and in another session also on a menu. After the fix, in two sessions with the pause menu and the map opened and closed many times (over 200 menu-frame samples in the logs, Hyrule Market included), it never froze and there were no driver faults after start. A later session with the output clearing ran as a first run (hookshot_driver_faults.txt removed). The driver faulted three times (exception 0xc0000005), and the precompile still completed in 2.6 s with no thread left in the driver. Pausing in Hyrule Market did not freeze either. This was tested on a v2.0.13 build with the same change backported; this branch is rebased on current main and merges cleanly.

Tests

  • New tests/test_driver_faults.py compiles the real driver_faults.h and runs it with threads:
    • the file's lines are filtered by vendor and driver;
    • a faulted program is refused afterwards;
    • a second build waits outside the driver and learns how the first one ended;
    • a cold probe gets kBusy;
    • with 8 threads on one program, never more than one is "in the driver".
  • It also checks in vk_draw.cpp that every guarded_graphics call of build_graphics (including #1747's retry) sits between the gate's begin and end, and that the guards stay self-contained for test_precompile_compute_guard.py and test_linux_driver_fault_guard.py.
  • tests/test_linux_driver_fault_guard.py now expects -77 for a graphics fault.
  • tests/test_linux_driver_fault_guard.py and tests/test_precompile_compute_guard.py: their fake drivers write a sentinel handle before they fault. The existing checks then assert that it is neither published nor destroyed. These run on Linux and on an MSVC-target build (CI).
  • test_driver_faults.py also covers the non-serial mode (builds side by side, never waiting nor busy, a fault still quarantined), and checks that every fault branch clears the output, and that every driver call of build_graphics (graphics_with included) is inside the gate.
  • Full run_all.py on Windows, rebased on current main: 291 passed. The 6 failures fail the same way on a clean main here (local environment).

Not covered

  • Why AMD's compiler faults on these three programs: this PR stops the fault from freezing the game, it does not make those draws work.
  • The other failures collected in #996 (TDRs, device lost in the Deku Tree, and so on) are different and not addressed here.
  • A program sharing a shader stage with a faulted one could in principle meet the same stuck compile. The dump showed only the same programs.
  • Compute pipelines keep their existing handling (-77, failed once); only their guards' output clearing is new.
Fixes #1740 Fixes #1734 Related to #996: the freeze on closing the pause menu or map, with the music still playing. ## The freeze On AMD Windows (driver 0x800161), the game freezes when closing the pause menu or the map in Kokiri Forest and Hyrule Market, with the music still playing. There is no TDR and no device lost. I took a full process dump of the freeze with Task Manager (RX 6800 XT, v2.0.13, closing the pause menu in Hyrule Market). I then read the thread stacks with the symbols in `hookshot.exe` / `hookshot_vk.dll`: | Thread | Where it was | |---|---| | `sead::MainThread` | `event_wait` (waiting for the frame) | | `Presentation Thread` | `mkwvk::draw` → `targets` → `jobs_wait`, for program **6eaa646d07ea4e43**'s pipeline | | `mkw_vk worker` | `graphics_pipeline` job → `build_graphics` → `guarded_graphics` → `amdvlk64.dll`, `SleepConditionVariableCS`, program **6eaa646d07ea4e43** | | precompile thread | `precompile_run` → `build_graphics` → `amdvlk64.dll`, `SleepConditionVariableCS`, program **6eaa646d07ea4e43** | | precompile thread | same, program **5fa56485a6b07619** | I found the program keys by following the `Program&` on each stack. Both programs are among the ones every AMD log reports at start as `graphics pipeline for 5fa56485a6b07619 / 6eaa646d07ea4e43 / 4e5ce183a33b0855 not made (-13)`. That `-13` is the `__except` in `guarded_graphics`, which catches an access violation inside AMD's compiler. Those two stuck precompile threads are also the `precompile: N of N+2 done, nothing finished for 30 s` of every AMD start. **What happens:** the caught fault leaves that program's compile unfinished inside the driver. Every later `vkCreateGraphicsPipelines` of the same program, with any state, waits inside the driver for a condition variable that is never signalled. The game runs until it needs a new pipeline of one of those programs (closing the pause menu, some areas), and then the frame never finishes. ## The fix `gpu/vk/driver_faults.h` (new, Vulkan-free) and `build_graphics`: - **Quarantine:** a program the driver faulted on is not asked of it again for the rest of the run. Its draws are left out, as they already were since its pipeline was never made. - **One build of a program in the driver at a time, on AMD:** a second build of the same program waits in Hookshot's gate, outside the driver. If the first one faulted, the second is refused instead of being caught inside the driver behind it. This makes the first run safe too. A cold probe (`FAIL_ON_PIPELINE_COMPILE_REQUIRED`) is never made to wait: a build of its program under way answers it as a compile that is required. This gate is on for AMD (vendor `0x1002`) by default, where the hang was seen. Other drivers build a program's state variants side by side, as before, and only the quarantine applies to them. - **Across runs:** the faults are written to `hookshot_driver_faults.txt` beside the executable, as `<vendor> <driver version> <key>` lines. On later runs with the same vendor and driver those programs are never asked of the driver, the precompile included. A driver update tries them again. If the file cannot be written (a read-only folder), the log says so once and the quarantine lasts for that run. - **Telling a fault apart:** the graphics guards (Windows SEH and Linux signal) now answer a fault with `-77`, as the compute guards already did. A driver's own `VK_ERROR_UNKNOWN` is no longer taken for a fault. The Windows guards keep #1766's log line with the exception code. - **No stray handle after a fault** (suggested by @Arkex-ai): on the caught-fault branch only, all four guards set `*pl = VK_NULL_HANDLE`. Whatever the driver wrote before it faulted is never published or destroyed; for example, `fallback_compute_pipelines()` destroys every non-null output on an error. - **Switch:** `RX_VK_DRIVER_FAULTS=0` turns all of this off; `=2` turns the gate on for every driver. **With #1747:** its AMD retry without the optimizer followed `VK_ERROR_UNKNOWN`, which until now also meant a fault. It still runs after a fault, now inside the gate. The program is quarantined only if the retry faults too. **With #1772:** the gate wraps `graphics_with`. That can reach the driver up to three times through the play caches, all inside the gate. `create_cached` returns the guard's result unchanged, so a fault still reaches the quarantine. ## Results (RX 6800 XT, driver 0x800161, Windows 11) The builder's precompile runs on the player's GPU, so the build log shows the effect directly: | | Driver faults | Precompile threads stuck forever | Precompile | |---|---|---|---| | Before | 3 | 2 | 32.5 s, `6625 of 6627 done, nothing finished for 30 s` | | First run with the fix | 3, quarantined; the second builds of 5fa56485 and 6eaa646d refused outside the driver | 0 | 4.2 s, complete | | Next runs | 0 (3 programs skipped from the file) | 0 | 2.3 s, complete | In game, before the fix, it froze on closing the pause menu in Hyrule Market, and in another session also on a menu. After the fix, in two sessions with the pause menu and the map opened and closed many times (over 200 menu-frame samples in the logs, Hyrule Market included), it never froze and there were no driver faults after start. A later session with the output clearing ran as a first run (`hookshot_driver_faults.txt` removed). The driver faulted three times (`exception 0xc0000005`), and the precompile still completed in 2.6 s with no thread left in the driver. Pausing in Hyrule Market did not freeze either. This was tested on a v2.0.13 build with the same change backported; this branch is rebased on current `main` and merges cleanly. ## Tests - New `tests/test_driver_faults.py` compiles the real `driver_faults.h` and runs it with threads: - the file's lines are filtered by vendor and driver; - a faulted program is refused afterwards; - a second build waits outside the driver and learns how the first one ended; - a cold probe gets `kBusy`; - with 8 threads on one program, never more than one is "in the driver". - It also checks in `vk_draw.cpp` that every `guarded_graphics` call of `build_graphics` (including #1747's retry) sits between the gate's `begin` and `end`, and that the guards stay self-contained for `test_precompile_compute_guard.py` and `test_linux_driver_fault_guard.py`. - `tests/test_linux_driver_fault_guard.py` now expects `-77` for a graphics fault. - `tests/test_linux_driver_fault_guard.py` and `tests/test_precompile_compute_guard.py`: their fake drivers write a sentinel handle before they fault. The existing checks then assert that it is neither published nor destroyed. These run on Linux and on an MSVC-target build (CI). - `test_driver_faults.py` also covers the non-serial mode (builds side by side, never waiting nor busy, a fault still quarantined), and checks that every fault branch clears the output, and that every driver call of `build_graphics` (`graphics_with` included) is inside the gate. - Full `run_all.py` on Windows, rebased on current `main`: 291 passed. The 6 failures fail the same way on a clean `main` here (local environment). ## Not covered - Why AMD's compiler faults on these three programs: this PR stops the fault from freezing the game, it does not make those draws work. - The other failures collected in #996 (TDRs, device lost in the Deku Tree, and so on) are different and not addressed here. - A program sharing a shader stage with a faulted one could in principle meet the same stuck compile. The dump showed only the same programs. - Compute pipelines keep their existing handling (`-77`, `failed` once); only their guards' output clearing is new.
hckrman101 commented 2026-11-10 17:51:02 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
Arkex-ai commented 2026-11-10 18:31:46 +00:00 (Migrated from github.com)

One adjacent fault-containment question from a source audit: the Windows and POSIX guards return the caught-fault result but leave *pl as the driver last wrote it. fallback_compute_pipelines() destroys every non-null entry in made on an error. A fault after writing an output could therefore send an untrusted handle back into vkDestroyPipeline.

Would clearing the output specifically on the caught-fault branches fit this work? Normal Vulkan error returns should retain their ordinary output/cleanup contract. A focused regression could have the authored driver stub write a sentinel and then raise the exception/signal, checking that the caller never destroys or publishes it.

This is a source-derived question, not a reproduced real-driver crash or a claim that the current quarantine change causes one. Sharing here because the guards are already in your scope; I have not started a competing implementation.

One adjacent fault-containment question from a source audit: the Windows and POSIX guards return the caught-fault result but leave `*pl` as the driver last wrote it. `fallback_compute_pipelines()` destroys every non-null entry in `made` on an error. A fault after writing an output could therefore send an untrusted handle back into `vkDestroyPipeline`. Would clearing the output specifically on the caught-fault branches fit this work? Normal Vulkan error returns should retain their ordinary output/cleanup contract. A focused regression could have the authored driver stub write a sentinel and then raise the exception/signal, checking that the caller never destroys or publishes it. This is a source-derived question, not a reproduced real-driver crash or a claim that the current quarantine change causes one. Sharing here because the guards are already in your scope; I have not started a competing implementation.
getzabelz commented 2026-11-10 18:49:14 +00:00 (Migrated from github.com)

@hckrman101 Rebased on current main (51de681). Nothing in the fix changed, but two merges since touched the same code:

  • #1766 (the Windows guards log the fault with its exception code): kept. The guards now log it and answer -77.
  • #1772 (graphics_with, through the play caches): build_graphics' gate now wraps graphics_with. That can reach the driver up to three times (generation cache, pcache, compile), all inside the gate, and create_cached returns the guard's result unchanged, so a fault still reaches the quarantine. #1747's AMD retry is still inside the gate too.

@Arkex-ai Good catch, and it fits here. All four guards (Windows SEH and Linux signal, graphics and compute) now set *pl = VK_NULL_HANDLE on the caught-fault branch only; a normal Vulkan error return keeps its usual contract. For the regression: the fake drivers in test_precompile_compute_guard.py and test_linux_driver_fault_guard.py now write a sentinel handle (0xdead) before they fault. Their existing checks then assert that it is neither published nor destroyed (out == 0, compute_pipeline == 0 && destroys == 0, pl == VK_NULL_HANDLE). test_driver_faults.py also checks that every fault branch clears the output.

Played again (RX 6800 XT, driver 0x800161, this change on a v2.0.13 build). I removed hookshot_driver_faults.txt first so the start was a first run:

  • the driver faulted three times in the precompile (exception 0xc0000005, #1766's line);
  • each output was cleared and its program quarantined;
  • the second builds of 5fa56485 and 6eaa646d were refused outside the driver;
  • the precompile completed in 2.6 s, with no thread left inside the driver;
  • the file was written again with the same three programs.

Then, in Hyrule Market, I opened and closed the pause menu and the map with no freeze and no further driver fault.

Side note: #1766's description mentions a start that hung on an RTX 4070 Ti after a precompile worker's build faulted inside the driver. That looks like the same mechanism as here, so the quarantine may help on NVIDIA too.

Full run_all.py on Windows: 270 passed. The 8 failures fail the same way on a clean main here (local environment). The two guard tests run only on Linux and on an MSVC-target build, so CI will run them.

@hckrman101 Rebased on current `main` (51de681). Nothing in the fix changed, but two merges since touched the same code: - **#1766** (the Windows guards log the fault with its exception code): kept. The guards now log it and answer `-77`. - **#1772** (`graphics_with`, through the play caches): `build_graphics`' gate now wraps `graphics_with`. That can reach the driver up to three times (generation cache, `pcache`, compile), all inside the gate, and `create_cached` returns the guard's result unchanged, so a fault still reaches the quarantine. #1747's AMD retry is still inside the gate too. @Arkex-ai Good catch, and it fits here. All four guards (Windows SEH and Linux signal, graphics and compute) now set `*pl = VK_NULL_HANDLE` on the caught-fault branch only; a normal Vulkan error return keeps its usual contract. For the regression: the fake drivers in `test_precompile_compute_guard.py` and `test_linux_driver_fault_guard.py` now write a sentinel handle (`0xdead`) before they fault. Their existing checks then assert that it is neither published nor destroyed (`out == 0`, `compute_pipeline == 0 && destroys == 0`, `pl == VK_NULL_HANDLE`). `test_driver_faults.py` also checks that every fault branch clears the output. **Played again** (RX 6800 XT, driver 0x800161, this change on a v2.0.13 build). I removed `hookshot_driver_faults.txt` first so the start was a first run: - the driver faulted three times in the precompile (`exception 0xc0000005`, #1766's line); - each output was cleared and its program quarantined; - the second builds of 5fa56485 and 6eaa646d were refused outside the driver; - the precompile completed in 2.6 s, with no thread left inside the driver; - the file was written again with the same three programs. Then, in Hyrule Market, I opened and closed the pause menu and the map with no freeze and no further driver fault. Side note: #1766's description mentions a start that hung on an RTX 4070 Ti after a precompile worker's build faulted inside the driver. That looks like the same mechanism as here, so the quarantine may help on NVIDIA too. Full `run_all.py` on Windows: 270 passed. The 8 failures fail the same way on a clean `main` here (local environment). The two guard tests run only on Linux and on an MSVC-target build, so CI will run them.
hckrman101 (Migrated from github.com) reviewed 2026-11-10 20:51:43 +00:00
hckrman101 (Migrated from github.com) left a comment

Nice find with the dump; the quarantine plus the gate is a clean way to stop the freeze, and the clearing of *pl on all four caught-fault branches covers Arkex-ai's point. I read the diff and the test and the logic holds: begin/end are paired with no early return between them in build_graphics, the -77 change keeps #1747's retry (VK_ERROR_UNKNOWN || kDriverFault), and the cold probe never blocks.

One question, not a blocker: the gate is on by default for every vendor, and it serializes all builds of one program (building_ is keyed by p.key alone, not by key plus state). Before, NVIDIA/Intel/Linux drivers compiled several state variants of the same program in parallel (precompile threads + worker), and a non-cold build on the presentation thread now also waits for a precompile thread that happens to be building the same program. The AMD hang needs the serialization, but other drivers don't. Did you see any change in precompile time or a frame stall on a non-AMD GPU? If it is measurable, you could limit the gate to the vendor/driver where it is needed (or only serialize once a program has faulted), and keep RX_VK_DRIVER_FAULTS as the override. The quarantine itself is vendor-neutral and cheap.

Small notes:

  • CI has not run on this head (the three workflows are action_required, waiting for approval), so the two guard tests and the new one are untested by CI so far.
  • hookshot_driver_faults.txt goes beside the executable; if that folder is read-only (Program Files) driver_fault_note fails silently, so the quarantine then only lasts for the run. That is fine, maybe worth one log line (nit).
Nice find with the dump; the quarantine plus the gate is a clean way to stop the freeze, and the clearing of `*pl` on all four caught-fault branches covers Arkex-ai's point. I read the diff and the test and the logic holds: `begin`/`end` are paired with no early return between them in `build_graphics`, the `-77` change keeps #1747's retry (`VK_ERROR_UNKNOWN || kDriverFault`), and the cold probe never blocks. One question, not a blocker: the gate is on by default for every vendor, and it serializes all builds of one program (`building_` is keyed by `p.key` alone, not by key plus state). Before, NVIDIA/Intel/Linux drivers compiled several state variants of the same program in parallel (precompile threads + worker), and a non-cold build on the presentation thread now also waits for a precompile thread that happens to be building the same program. The AMD hang needs the serialization, but other drivers don't. Did you see any change in precompile time or a frame stall on a non-AMD GPU? If it is measurable, you could limit the gate to the vendor/driver where it is needed (or only serialize once a program has faulted), and keep `RX_VK_DRIVER_FAULTS` as the override. The quarantine itself is vendor-neutral and cheap. Small notes: - CI has not run on this head (the three workflows are `action_required`, waiting for approval), so the two guard tests and the new one are untested by CI so far. - `hookshot_driver_faults.txt` goes beside the executable; if that folder is read-only (Program Files) `driver_fault_note` fails silently, so the quarantine then only lasts for the run. That is fine, maybe worth one log line (nit).
getzabelz commented 2026-11-10 22:34:47 +00:00 (Migrated from github.com)

@hckrman101 Thanks, good point. I went with your first suggestion. New commit on top: dd68029 (rebased on current main).

  • The gate is now AMD-only by default (vendor 0x1002). NVIDIA, Intel and the Linux drivers build a program's state variants side by side again, exactly as before this PR. The quarantine stays on for every vendor, as you suggested.
  • RX_VK_DRIVER_FAULTS=2 turns the gate on for every driver, =0 still turns everything off. #1766's description mentions a start that hung on an RTX 4070 Ti after a driver fault, so someone on NVIDIA can try it without a rebuild.
  • I didn't go with "serialize only after a program has faulted": in the dump, the second build was already inside the driver when the first one faulted, so it would not have helped the first run, which is exactly when it hangs.
  • I can't measure the non-AMD cost myself (I only have the RX 6800 XT), which is the other reason to keep it to AMD.
  • The faults file: if hookshot_driver_faults.txt can't be written (read-only folder), the log now says so once, and the quarantine lasts for that run.
  • Tests: test_driver_faults.py now also covers the non-serial mode. Builds of one program go into the driver side by side, never wait nor get kBusy, and a faulted program is still kept out. Full run_all.py on Windows: 291 passed; the 6 failures fail the same way on a clean main here.

Played on a build of current main with both commits (RX 6800 XT):

  • the precompile completed (52,940 programs, 5,980 graphics and 1,240 compute pipelines) with no thread stuck in the driver;
  • the log shows the gate on for AMD;
  • opening and closing the pause menu and inventory many times did not freeze.

Also from earlier play: a new program (86068cde6ce03d5d) faulted in the driver mid-game, not at start. It was caught, quarantined and written to the file, and the game carried on without freezing.

@hckrman101 Thanks, good point. I went with your first suggestion. New commit on top: dd68029 (rebased on current `main`). - **The gate is now AMD-only by default** (vendor `0x1002`). NVIDIA, Intel and the Linux drivers build a program's state variants side by side again, exactly as before this PR. The **quarantine stays on for every vendor**, as you suggested. - **`RX_VK_DRIVER_FAULTS=2`** turns the gate on for every driver, `=0` still turns everything off. #1766's description mentions a start that hung on an RTX 4070 Ti after a driver fault, so someone on NVIDIA can try it without a rebuild. - **I didn't go with "serialize only after a program has faulted"**: in the dump, the second build was already inside the driver when the first one faulted, so it would not have helped the first run, which is exactly when it hangs. - **I can't measure the non-AMD cost myself** (I only have the RX 6800 XT), which is the other reason to keep it to AMD. - **The faults file:** if `hookshot_driver_faults.txt` can't be written (read-only folder), the log now says so once, and the quarantine lasts for that run. - **Tests:** `test_driver_faults.py` now also covers the non-serial mode. Builds of one program go into the driver side by side, never wait nor get `kBusy`, and a faulted program is still kept out. Full `run_all.py` on Windows: 291 passed; the 6 failures fail the same way on a clean `main` here. **Played on a build of current `main`** with both commits (RX 6800 XT): - the precompile completed (52,940 programs, 5,980 graphics and 1,240 compute pipelines) with no thread stuck in the driver; - the log shows the gate on for AMD; - opening and closing the pause menu and inventory many times did not freeze. Also from earlier play: a **new** program (`86068cde6ce03d5d`) faulted in the driver mid-game, not at start. It was caught, quarantined and written to the file, and the game carried on without freezing.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 01:37:55 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the follow-up, this answers my question.

  • The gate is now serial only on AMD (vendor 0x1002) or with RX_VK_DRIVER_FAULTS=2, and other drivers build a program's variants side by side again; the quarantine stays vendor-neutral. In non-serial mode begin never waits or answers kBusy, and end only erases a key that was never inserted, so nothing changes for NVIDIA/Intel apart from the quarantine. The new not_serial() test covers that.
  • Your reason for not doing "serialize only after a fault" makes sense: in the dump the second build was already inside the driver when the first one faulted, so that would not save the first run.
  • The read-only folder case now logs once (the nit from last time).
  • @Arkex-ai's point about stale handles after a fault was already covered in the previous push (all four guards clear *pl), and it is still in place.
  • CI ran this time. The only failure is tests/test_log_batch.py on windows-2022 (GetTickCount64() - t0 < 200, a timing check). This PR doesn't touch logging, so I'd treat it as a flaky runner. test_driver_faults.py and test_precompile_compute_guard.py pass on Windows, and the Ubuntu job (with the Linux guard test) is green.

One small optional note inline. Good to go from my side.

Thanks for the follow-up, this answers my question. - The gate is now serial only on AMD (vendor `0x1002`) or with `RX_VK_DRIVER_FAULTS=2`, and other drivers build a program's variants side by side again; the quarantine stays vendor-neutral. In non-serial mode `begin` never waits or answers `kBusy`, and `end` only erases a key that was never inserted, so nothing changes for NVIDIA/Intel apart from the quarantine. The new `not_serial()` test covers that. - Your reason for not doing "serialize only after a fault" makes sense: in the dump the second build was already inside the driver when the first one faulted, so that would not save the first run. - The read-only folder case now logs once (the nit from last time). - @Arkex-ai's point about stale handles after a fault was already covered in the previous push (all four guards clear `*pl`), and it is still in place. - CI ran this time. The only failure is `tests/test_log_batch.py` on windows-2022 (`GetTickCount64() - t0 < 200`, a timing check). This PR doesn't touch logging, so I'd treat it as a flaky runner. `test_driver_faults.py` and `test_precompile_compute_guard.py` pass on Windows, and the Ubuntu job (with the Linux guard test) is green. One small optional note inline. Good to go from my side.
gpu/vk/vk_draw.cpp
@ -1014,1 +1015,4 @@
// (hookshot) The programs the driver faulted on (driver_faults.h): not asked of it again in this run, nor in later runs on the
// same driver (hookshot_driver_faults.txt beside the executable). One program's builds go to the driver one at a time on AMD,
// where a build behind a faulted one hung (issue 996); other drivers keep building a program's variants in parallel.
hckrman101 (Migrated from github.com) commented 2026-11-11 01:37:55 +00:00

Nit, optional: vendorID == 0x1002 also turns the gate on for RADV on Linux, and RADV is what most Linux AMD players run. The hang was seen with amdvlk64 on Windows. If you want to narrow it, VkPhysicalDeviceDriverProperties::driverID == VK_DRIVER_ID_AMD_PROPRIETARY (plus AMD_OPEN_SOURCE for AMDVLK on Linux) would leave RADV out. The cost there is small anyway (only variants of one program are serialized), so fine to leave as is.

Nit, optional: `vendorID == 0x1002` also turns the gate on for RADV on Linux, and RADV is what most Linux AMD players run. The hang was seen with amdvlk64 on Windows. If you want to narrow it, `VkPhysicalDeviceDriverProperties::driverID == VK_DRIVER_ID_AMD_PROPRIETARY` (plus `AMD_OPEN_SOURCE` for AMDVLK on Linux) would leave RADV out. The cost there is small anyway (only variants of one program are serialized), so fine to leave as is.
getzabelz commented 2026-11-11 08:32:17 +00:00 (Migrated from github.com)

@hckrman101 Thanks for the approval. Rebased on current main (d7a7306), now 3dc9b5d. The only conflict was in docs/development.md: the RX_VK_DRIVER_FAULTS row and #1992's RX_VK_TASK_MESH_FALLBACK row landed in the same spot, so I kept both. The code is unchanged.

On the RADV note: I'll leave the gate on vendorID == 0x1002 for now. As you said, the cost is small (only one program's variants are serialized), and I'd rather not narrow it to driverID without a RADV report either way. If it shows up in Linux profiles, it's a one-line follow-up.

@hckrman101 Thanks for the approval. Rebased on current `main` (d7a7306), now 3dc9b5d. The only conflict was in `docs/development.md`: the `RX_VK_DRIVER_FAULTS` row and #1992's `RX_VK_TASK_MESH_FALLBACK` row landed in the same spot, so I kept both. The code is unchanged. On the RADV note: I'll leave the gate on `vendorID == 0x1002` for now. As you said, the cost is small (only one program's variants are serialized), and I'd rather not narrow it to `driverID` without a RADV report either way. If it shows up in Linux profiles, it's a one-line follow-up.
getzabelz commented 2026-11-11 08:59:36 +00:00 (Migrated from github.com)

CI note: after the rebase, windows-2022, part 1/2 failed in tests/test_recorder_sync.py (the defer/thread lifecycle checks against the fake driver). This PR doesn't touch vk_work.h, vk_recorder.h or anything else that test builds. Locally, on the rebased branch, every lifecycle, handoff and allocation-failure check passes, with the same result as current main. It looks like a timing flake on the runner. I can't re-run jobs on this repo, so could someone with access re-run the failed job? Thanks.

CI note: after the rebase, `windows-2022, part 1/2` failed in `tests/test_recorder_sync.py` (the defer/thread lifecycle checks against the fake driver). This PR doesn't touch `vk_work.h`, `vk_recorder.h` or anything else that test builds. Locally, on the rebased branch, every lifecycle, handoff and allocation-failure check passes, with the same result as current `main`. It looks like a timing flake on the runner. I can't re-run jobs on this repo, so could someone with access re-run the failed job? Thanks.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 16:28:18 +00:00
hckrman101 (Migrated from github.com) left a comment

Posted twice by mistake; see my next review (at 3dc9b5d).

Posted twice by mistake; see my next review (at 3dc9b5d).
gpu/vk/vk_draw.cpp
@ -1014,1 +1015,4 @@
// (hookshot) The programs the driver faulted on (driver_faults.h): not asked of it again in this run, nor in later runs on the
// same driver (hookshot_driver_faults.txt beside the executable). One program's builds go to the driver one at a time on AMD,
// where a build behind a faulted one hung (issue 996); other drivers keep building a program's variants in parallel.
hckrman101 (Migrated from github.com) commented 2026-11-11 16:28:18 +00:00

Nit, optional: vendorID == 0x1002 also turns the gate on for RADV on Linux, and RADV is what most Linux AMD players run. The hang was seen with amdvlk64 on Windows. If you want to narrow it, VkPhysicalDeviceDriverProperties::driverID == VK_DRIVER_ID_AMD_PROPRIETARY (plus AMD_OPEN_SOURCE for AMDVLK on Linux) would leave RADV out. The cost there is small anyway (only variants of one program are serialized), so fine to leave as is.

Nit, optional: `vendorID == 0x1002` also turns the gate on for RADV on Linux, and RADV is what most Linux AMD players run. The hang was seen with amdvlk64 on Windows. If you want to narrow it, `VkPhysicalDeviceDriverProperties::driverID == VK_DRIVER_ID_AMD_PROPRIETARY` (plus `AMD_OPEN_SOURCE` for AMDVLK on Linux) would leave RADV out. The cost there is small anyway (only variants of one program are serialized), so fine to leave as is.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 16:28:50 +00:00
hckrman101 (Migrated from github.com) left a comment

Re-checked after the rebase onto d7a7306. I compared both commits file by file with the version I approved (dd68029). The added and removed lines are the same in every file, so the fix and the tests are unchanged. The only change is the docs/development.md conflict, where both the RX_VK_DRIVER_FAULTS and RX_VK_TASK_MESH_FALLBACK rows are kept. Fine.

Keeping the gate on vendorID == 0x1002 for now is fine with me, as you said. The RADV cost is small, and narrowing it to driverID can follow if Linux profiles show it.

CI: everything is green except windows-2022, part 1/2, which fails in tests/test_recorder_sync.py. This PR doesn't touch anything that test builds, and main passes it at the same base (d7a7306), so it looks like a runner flake. It needs a re-run by someone with access before merge.

Overlap: #2195 (which replaces #2123) changes the same four fault-guard blocks, to count the faults it catches. Its description already says that whichever lands second needs a mechanical rebase, and that after this one its count would follow the kDriverFault path. So the two don't conflict in design, only in text.

Still good to go from my side. (The review just above, at dd68029, was a duplicate of my previous one posted by mistake; this one is the current one.)

Re-checked after the rebase onto d7a7306. I compared both commits file by file with the version I approved (dd68029). The added and removed lines are the same in every file, so the fix and the tests are unchanged. The only change is the `docs/development.md` conflict, where both the `RX_VK_DRIVER_FAULTS` and `RX_VK_TASK_MESH_FALLBACK` rows are kept. Fine. Keeping the gate on `vendorID == 0x1002` for now is fine with me, as you said. The RADV cost is small, and narrowing it to `driverID` can follow if Linux profiles show it. CI: everything is green except `windows-2022, part 1/2`, which fails in `tests/test_recorder_sync.py`. This PR doesn't touch anything that test builds, and main passes it at the same base (d7a7306), so it looks like a runner flake. It needs a re-run by someone with access before merge. Overlap: #2195 (which replaces #2123) changes the same four fault-guard blocks, to count the faults it catches. Its description already says that whichever lands second needs a mechanical rebase, and that after this one its count would follow the `kDriverFault` path. So the two don't conflict in design, only in text. Still good to go from my side. (The review just above, at dd68029, was a duplicate of my previous one posted by mistake; this one is the current one.)
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
hckrman101
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
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/Hookshot!1823
No description provided.