Fix background shader and pipeline jobs left pending after exceptions #1874

Merged

SRhartty merged 1 commit from codex/complete-failed-background-jobs into main

2026-11-10 21:03:03 +00:00

SRhartty commented

2026-11-10 19:09:32 +00:00

(Migrated from github.com)

A C++ exception in shader translation or pipeline creation is caught by run_guarded, but can leave the resource permanently pending. Draws may then wait indefinitely in jobs_wait or keep skipping that resource.

This change adds optional nonthrowing failure callbacks to the shared and serial job queues. They publish the resource's existing failure/completion state before the original exception is logged and waiters are notified. Wrapping before enqueue also covers jobs executed directly by jobs_wait.

Shader translation marks the program failed before publishing readiness.
Graphics builds complete with a null pipeline on failure, while preserving an already usable linked pipeline.
Ordinary and mesh-fallback compute builds leave the compiling state on exception.

Validation:

Added game-free runtime regression tests for standard and non-standard exceptions on shared workers, the serial worker, and stolen jobs. Each checks waiter release and subsequent queue progress. The original code fails the regression.
Executed the production recovery callbacks against resource stand-ins, covering shader failure, null/linked graphics pipelines, fallback compute, and ordinary compute.
llvm-mingw C++20 syntax checks passed for all three modified implementation files using the Builder's headers and build definitions.
Full Windows suite: python -X utf8 -I -B tests/run_all.py --jobs 2: 253 passed, 17 failed, 51 skipped. All 17 failures were reproduced against unmodified base 51de681; no additional failures appeared. Skips reflect unavailable tools/dependencies or platform exclusions.

This handles C++ exceptions; it does not cancel Vulkan driver calls that never return or claim to fix the observed AMD pipeline-creation hang. The running game installation has not been replaced with this change.

A C++ exception in shader translation or pipeline creation is caught by `run_guarded`, but can leave the resource permanently pending. Draws may then wait indefinitely in `jobs_wait` or keep skipping that resource. This change adds optional nonthrowing failure callbacks to the shared and serial job queues. They publish the resource's existing failure/completion state before the original exception is logged and waiters are notified. Wrapping before enqueue also covers jobs executed directly by `jobs_wait`. - Shader translation marks the program failed before publishing readiness. - Graphics builds complete with a null pipeline on failure, while preserving an already usable linked pipeline. - Ordinary and mesh-fallback compute builds leave the compiling state on exception. Validation: - Added game-free runtime regression tests for standard and non-standard exceptions on shared workers, the serial worker, and stolen jobs. Each checks waiter release and subsequent queue progress. The original code fails the regression. - Executed the production recovery callbacks against resource stand-ins, covering shader failure, null/linked graphics pipelines, fallback compute, and ordinary compute. - llvm-mingw C++20 syntax checks passed for all three modified implementation files using the Builder's headers and build definitions. - Full Windows suite: `python -X utf8 -I -B tests/run_all.py --jobs 2`: **253 passed, 17 failed, 51 skipped**. All 17 failures were reproduced against unmodified base `51de681`; no additional failures appeared. Skips reflect unavailable tools/dependencies or platform exclusions. This handles C++ exceptions; it does not cancel Vulkan driver calls that never return or claim to fix the observed AMD pipeline-creation hang. The running game installation has not been replaced with this change.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 20:52:58 +00:00

hckrman101 (Migrated from github.com) left a comment

Nits, none blocking:

translate_into already catches std::exception itself (sets failed, ready, bumps st_failed), so the new shader callbacks mostly cover non-std throws and anything escaping that handler. They don't bump st_failed or log, so such a program won't show in the "N failed" count. Fine to leave; maybe worth a comment.
The compute failed lambda nulls q.compute_pipeline. If something threw after q.compute_pipeline = made, that pipeline handle would be leaked rather than reused. Practically unreachable (only logging/counters follow), so leave it or just don't touch compute_pipeline there.
tests/test_jobs_failure.py finds the recovery lambdas by [job]() noexcept { and hard-codes counts (2 in vk_shader.cpp, 3 in vk_draw.cpp). Adding a fourth callback or reformatting will trip it with a fairly cryptic message; fine, just be aware.

No CI check runs are reported on this head, so I couldn't compare against main's known failures; the PR's 17 failures reproduced on base are consistent with that. Good to go from my side.

Nice, tight fix. Wrapping before enqueue means the shared workers, the serial worker and jobs_wait's stolen jobs all get the same recovery, and the regression test exercises all three paths with std and non-std throws. I read each recovery against how its resource is consumed: shader failure (failed + ready, which the draw paths check), graphics (ready with a null pipeline hits the existing `if (!pl) SKIP_HERE()`, and a linked pipeline is kept), fallback compute (state 2 with empty fallback_compute reads as failed in fallback_pipelines_ready) and compute (failed + state 2 -> SKIP_HERE). All consistent. Nits, none blocking: - `translate_into` already catches `std::exception` itself (sets failed, ready, bumps st_failed), so the new shader callbacks mostly cover non-std throws and anything escaping that handler. They don't bump `st_failed` or log, so such a program won't show in the "N failed" count. Fine to leave; maybe worth a comment. - The compute `failed` lambda nulls `q.compute_pipeline`. If something threw after `q.compute_pipeline = made`, that pipeline handle would be leaked rather than reused. Practically unreachable (only logging/counters follow), so leave it or just don't touch compute_pipeline there. - `tests/test_jobs_failure.py` finds the recovery lambdas by `[job]() noexcept {` and hard-codes counts (2 in vk_shader.cpp, 3 in vk_draw.cpp). Adding a fourth callback or reformatting will trip it with a fairly cryptic message; fine, just be aware. No CI check runs are reported on this head, so I couldn't compare against main's known failures; the PR's 17 failures reproduced on base are consistent with that. Good to go from my side.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101