a73x

7ef4a62c

Add Vulkan bounded-waits design spec

a73x   2026-04-18 09:56

Commit message
Add Vulkan bounded-waits design spec

Tracks issue ab6c92f0. Replaces unbounded vkWaitForFences /
vkAcquireNextImageKHR / vkDeviceWaitIdle calls with bounded helpers
in src/vk_sync.zig + a grep-gate test. Skip-frame on timeout, no
GPU rebuild, no DEVICE_LOST handling (ticket scope deferred).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs/superpowers/specs/2026-04-18-vulkan-bounded-waits-design.md
Old New
@@ -0,0 +1,274 @@
1 # Vulkan Bounded Waits Design
2
3 ## Goal
4
5 Stop waystty from hanging when the Vulkan driver drops a fence signal. Replace every unbounded `vkWaitForFences` / `vkAcquireNextImageKHR` / `vkDeviceWaitIdle` / `vkQueueWaitIdle` call with bounded variants that return a recoverable error on timeout. On timeout, the main loop logs the event, skips the frame, and retries on the next iteration. The window stays responsive — keystrokes, resize, and exit continue to work — instead of wedging on the last-presented frame.
6
7 Tracks issue `ab6c92f0`. Closes `793f491a` as a duplicate.
8
9 ## Background
10
11 Two freezes were observed on the same daily-driver session (2026-04-18). Both wedged the main thread inside the NVIDIA 595.58.03 driver at:
12
13 ```
14 main.runTerminal
15 renderer.Context.drawCells
16 vk.DeviceWrapper.waitForFences(..., timeout=18446744073709551615)
17 libnvidia-glcore internal poll() ← wedged
18 ```
19
20 The driver appears to drop a kernel syncobj / sync-fd notification for `in_flight_fence`. The GPU work likely completes; the userspace poll never wakes. With `timeout=UINT64_MAX`, the main loop blocks permanently — no input dispatch, no resize, no exit.
21
22 The fix is straightforward: bounded timeouts on every wait, classified errors, frame-skip on timeout. This is not a recovery design — there is no GPU-resource rebuild, no `VK_ERROR_DEVICE_LOST` handling, no exit-after-N-failures. We have not observed `DEVICE_LOST` and we are not designing for it. Those remain open in the ticket as future scope.
23
24 ## Non-goals
25
26 - GPU-resource rebuild on timeout (swapchain, fences, pipeline, atlas, instance buffer).
27 - `VK_ERROR_DEVICE_LOST` handling. Treated as fatal if it ever fires; not observed.
28 - Recreating the `VkDevice`. Not observed.
29 - Exit-after-N-consecutive-timeouts policy. Deferred — if the driver wedges persistently, the main loop will keep retrying with backoff. The user can `Ctrl+C` or close the window because the loop is unblocked between waits.
30 - Auditing every existing `deviceWaitIdle` / `queueWaitIdle` for whether it should remain unbounded. The grep gate forces every site to declare intent (bounded or shutdown), but the per-site policy review is filed as a follow-up ticket.
31
32 ## Architecture
33
34 A new module `src/vk_sync.zig` exposes the bounded primitives. `Context` and `main.zig` call into this module instead of `self.vkd.waitForFences` etc. directly. A grep gate in the test target fails CI if any source file calls the underlying `vkd` methods outside `vk_sync.zig` itself.
35
36 Three concerns separated:
37
38 1. **The bounded helpers.** Five free functions, taking `vkd`, the `device` handle, and the operation-specific args. Stateless, easy to unit test.
39 2. **The fence-state invariant.** A documented contract about the state of `in_flight_fence` after each error return from `drawCells` / `drawClear` / `renderToOffscreen`, enforced by the order of operations in those functions.
40 3. **The timeout observability layer.** A counter + rate-limited `std.log.warn` call so persistent timeouts surface without spamming the log.
41
42 ## Module 1: `src/vk_sync.zig`
43
44 Five public functions plus two error definitions and two timeout constants.
45
46 ```zig
47 pub const fence_wait_timeout_ns: u64 = 2_000_000_000; // 2 s
48 pub const acquire_timeout_ns: u64 = 100_000_000; // 100 ms
49
50 pub const SyncError = error{ VkWaitTimeout, VkAcquireTimeout };
51
52 /// Bounded fence wait. On timeout, returns error.VkWaitTimeout without
53 /// touching the fence. The fence remains in whatever state it was — caller
54 /// can safely retry the wait on the next iteration.
55 pub fn waitFenceBounded(vkd: vk.DeviceWrapper, device: vk.Device, fence: vk.Fence) !void;
56
57 /// Bounded image acquire. Returns the acquired image_index on success.
58 /// Folds VK_SUBOPTIMAL_KHR into error.OutOfDateKHR (matches existing caller
59 /// semantics, which already collapse the two via swapchainNeedsRebuild).
60 /// Returns error.VkAcquireTimeout on timeout.
61 pub fn acquireImageBounded(
62 vkd: vk.DeviceWrapper,
63 device: vk.Device,
64 swapchain: vk.SwapchainKHR,
65 semaphore: vk.Semaphore,
66 ) !u32;
67
68 /// Bounded device idle. For uses where blocking forever is wrong (recovery
69 /// arms, mid-frame resync, etc.). Returns error.VkWaitTimeout on timeout.
70 pub fn waitIdleBounded(vkd: vk.DeviceWrapper, device: vk.Device, timeout_ns: u64) !void;
71
72 /// Unbounded device idle, named to make intent obvious at the call site.
73 /// Use only for shutdown drains where blocking forever is acceptable.
74 /// The grep gate exempts this function so it is the canonical way to write
75 /// an intentional unbounded wait.
76 pub fn waitIdleForShutdown(vkd: vk.DeviceWrapper, device: vk.Device) void;
77
78 /// Bounded queue idle. Same shape as waitIdleBounded.
79 pub fn queueWaitIdleBounded(vkd: vk.DeviceWrapper, queue: vk.Queue, timeout_ns: u64) !void;
80 ```
81
82 Why functions and not a wrapper struct: see "Approach: helpers, not dispatch wrapper" below.
83
84 `waitIdleForShutdown` returns `void` (not `!void`) because the only legitimate failure modes (`VK_ERROR_DEVICE_LOST` during shutdown) are unactionable — log internally and return.
85
86 ## Module 2: Caller updates
87
88 Ten call sites are migrated. All eight `waitForFences` and both `acquireNextImageKHR` calls in waystty source today, taken from a fresh grep (the original ticket missed three of them):
89
90 | Site | File | Lines | New helper |
91 |---|---|---|---|
92 | `drawClear` fence wait | `src/renderer.zig` | 1275 | `waitFenceBounded` |
93 | `drawClear` acquire | `src/renderer.zig` | 1279 | `acquireImageBounded` |
94 | `uploadAtlasRegion` fence | `src/renderer.zig` | 1478 | `waitFenceBounded` |
95 | `drawCells` fence wait | `src/renderer.zig` | 1710 | `waitFenceBounded` |
96 | `drawCells` acquire | `src/renderer.zig` | 1718 | `acquireImageBounded` |
97 | `renderToOffscreen` first wait | `src/renderer.zig` | 1818 | `waitFenceBounded` |
98 | `renderToOffscreen` capture wait | `src/renderer.zig` | 1824 | `waitFenceBounded` |
99 | `renderToOffscreen` post-submit wait | `src/renderer.zig` | 1941 | `waitFenceBounded` |
100 | `drawTextCoverageCompareFrame` fence | `src/main.zig` | 1364 | `waitFenceBounded` |
101 | `drawTextCoverageCompareFrame` acquire | `src/main.zig` | 1373 | `acquireImageBounded` |
102
103 The 13 `deviceWaitIdle` and 1 `queueWaitIdle` sites (14 total) are migrated mechanically to `waitIdleForShutdown` (preserving existing unbounded behavior). Choosing the *named* shutdown helper documents intent at the call site without changing semantics. A follow-up ticket (filed as part of this work) audits whether any of those should become `waitIdleBounded` instead — the recovery arm at `main.zig:681` is the most suspicious site.
104
105 ## Module 3: Fence-state invariant + reorder fix
106
107 Today, in both `drawClear` and `drawCells`:
108
109 ```zig
110 try waitForFences(in_flight_fence, UINT64_MAX); // line 1275 / 1710
111 try resetFences(in_flight_fence); // line 1276 / 1711
112 const acquire = try acquireNextImageKHR(...); // line 1282 / 1721
113 ```
114
115 `resetFences` runs *before* the acquire. With newly-possible acquire timeouts, this introduces a deadlock: if acquire returns `error.VkAcquireTimeout`, the fence is left unsignaled with no submit pending. Every subsequent fence wait then times out forever — the GPU has nothing in flight that will signal it.
116
117 Fix: move `resetFences` to *after* a successful acquire and before the submit.
118
119 ```zig
120 try vk_sync.waitFenceBounded(...);
121 const image_index = try vk_sync.acquireImageBounded(...);
122 try vkd.resetFences(...); // moved here
123 // ... submit, present
124 ```
125
126 This is sync-safe because `in_flight_fence` tracks submit completion (not acquire); acquire→submit ordering is handled by `image_available` semaphore.
127
128 After the reorder, the fence-state invariant per error return is documented at the top of `drawCells` and `drawClear`. **Contract:** the fence is *only* touched (reset, re-signaled) on the post-acquire failure paths — see "Post-acquire failure: fence re-signal" below. On `VkWaitTimeout` and `VkAcquireTimeout`, no GPU state is touched.
129
130 | Error returned by `drawCells` / `drawClear` | `in_flight_fence` state |
131 |---|---|
132 | `error.VkWaitTimeout` (from `waitFenceBounded`) | Whatever it was: signaled, or unsignaled with prior-frame submit still in flight. Next iteration's wait is safe. |
133 | `error.VkAcquireTimeout` (from `acquireImageBounded`) | Same as before this frame — reset has not run. Safe. |
134 | `error.OutOfDateKHR` post-acquire, pre-submit | See "Post-acquire failure" below — fence re-signaled before return. |
135 | `error.OutOfDateKHR` post-submit (from present) | Signaled by GPU on submit completion. Caller's existing `deviceWaitIdle` + rebuild drains it. |
136 | Success | Signaled by GPU when submit completes; reset at the top of the next frame. |
137
138 ### Post-acquire failure: fence re-signal
139
140 If `acquireImageBounded` succeeds and `resetFences` runs, but a subsequent operation fails before `queueSubmit` (the command-buffer recording calls between `resetFences` at line 1711 and `queueSubmit` at line 1773 in current `drawCells` — `resetCommandBuffer`, `beginCommandBuffer`, `cmdBeginRenderPass`, `recordDrawCommands`, `cmdEndRenderPass`, `endCommandBuffer`), the fence is unsignaled with no submit pending. To preserve the invariant, `drawCells` / `drawClear` re-signal the fence before returning. Concretely: in the `errdefer` path between `resetFences` and `queueSubmit`, submit a no-op (empty submit info with `signal_fence = in_flight_fence`) to put the fence back in the signaled state. Implementation note for the plan: this is one helper invocation, not five lines per site.
141
142 **`vkResetFences` failure**: per the Vulkan spec, the only failure mode for `vkResetFences` is `VK_ERROR_OUT_OF_DEVICE_MEMORY` — extremely rare. If this happens, the fence is in an undefined state and an image has been acquired with no submit pending. Treat as fatal: propagate the error from `drawCells` / `drawClear`, do not attempt re-signal (the device is in a degraded state and a no-op submit is unlikely to succeed). The main loop will see this as a non-recoverable error and exit.
143
144 `renderToOffscreen` has a related but different issue: it waits on `in_flight_fence` *before* `uploadInstances` to prevent host-overwrites of `instance_memory` while the GPU is still reading. With timeouts, the wait can return early. The function must propagate `error.VkWaitTimeout` *before* calling `uploadInstances`. Order is preserved (`waitFenceBounded` → check error → `uploadInstances`); no re-signal needed since `renderToOffscreen` doesn't reset `in_flight_fence`.
145
146 ## Module 4: Atlas upload timeout
147
148 `uploadAtlasRegion` waits on `atlas_transfer_fence` before recording the upload command buffer. On timeout, the function returns `error.VkWaitTimeout` without modifying any GPU state.
149
150 The atlas region's CPU-side dirty flag is left set automatically by the existing call structure: in `main.zig:591–605`, `atlas.dirty = false` runs *after* `try ctx.uploadAtlasRegion(...)`, so the `try` propagates the timeout error before the dirty flag is cleared. (`font.Atlas.dirty` is just a `bool` — there is no explicit retry mechanism in the Atlas struct itself; the survival depends on `try`-propagates-before-side-effects ordering in the caller.) On the next render iteration, the upload retries. If the underlying driver wedge persists, `uploadAtlasRegion` will keep timing out and `drawCells` will keep skipping frames; this is the same outcome as a `drawCells` fence timeout.
151
152 Caller (in `main.zig`) treats `error.VkWaitTimeout` from atlas upload identically to `drawCells` timeout: log via `logVkTimeout`, skip the render iteration, mark frame dirty.
153
154 ## Module 5: Logging
155
156 Single function in `src/vk_sync.zig`:
157
158 ```zig
159 var vk_timeout_count: std.atomic.Value(u64) = .init(0);
160 var last_log_ns: std.atomic.Value(i128) = .init(0);
161
162 const TimeoutKind = enum { fence, acquire, atlas };
163
164 pub fn logVkTimeout(src: std.builtin.SourceLocation, kind: TimeoutKind) void {
165 const n = vk_timeout_count.fetchAdd(1, .monotonic) + 1;
166 const now = std.time.nanoTimestamp();
167 const last = last_log_ns.load(.monotonic);
168 if (n == 1 or (now - last) > 5 * std.time.ns_per_s) {
169 last_log_ns.store(now, .monotonic);
170 std.log.warn("vk timeout #{} ({s}) at {s}:{d} — driver may be wedged",
171 .{ n, @tagName(kind), src.file, src.line });
172 }
173 }
174 ```
175
176 First occurrence logs immediately. Subsequent occurrences log at most once per 5 seconds with a running count. This makes a one-off flake visible (one line, no spam) and a persistent wedge observable (one line every 5 s with a growing counter), without flooding stderr in either case.
177
178 The 5-second window is a heuristic — short enough that "is it still happening?" is answerable within a few seconds, long enough that a 60Hz wedge produces ~1 log line per 5 s instead of ~300.
179
180 ## Module 6: Backoff on persistent timeout
181
182 In `main.zig`, the timeout arm tracks `consecutive_vk_timeouts: u32`:
183
184 ```zig
185 error.VkWaitTimeout, error.VkAcquireTimeout => {
186 vk_sync.logVkTimeout(@src(), .fence);
187 consecutive_vk_timeouts += 1;
188 const backoff_us = @min(consecutive_vk_timeouts * 5_000, 100_000); // cap 100 ms
189 std.time.sleep(backoff_us * std.time.ns_per_us);
190 render_pending = true;
191 continue;
192 },
193 // On any successful drawCells:
194 consecutive_vk_timeouts = 0;
195 ```
196
197 5 ms first miss, 10 ms second, … capped at 100 ms after ~20 consecutive misses. Prevents a tight retry loop from burning a CPU core when the GPU is genuinely wedged. No exit, no recovery — just don't spin.
198
199 **Frame-loop interaction**: the `std.time.sleep` runs inside the timeout-arm `continue` path, before the next iteration's `waitFrame` call. It does not interact with `frame_loop.zig`'s `pending_token` / `armed` state — the sleep simply delays re-entry into the main loop, after which the normal frame-loop gating runs as usual. No `forceArm` needed; the sleep is orthogonal.
200
201 ## Approach: helpers, not dispatch wrapper
202
203 We considered three consolidation strategies:
204
205 - **(α) Inline pattern.** Same code repeated at each site. Rejected — original ticket missed three sites; a structural fix is needed.
206 - **(β) Free-function helpers + grep gate.** Adopted.
207 - **(γ) Dispatch-table wrapper.** Wrap `vk.DeviceWrapper` in a struct that hides timeout-accepting methods. Rejected after research.
208
209 Research into production Vulkan projects found no precedent for γ. wgpu (`wgpu-hal/src/vulkan/device.rs`) and Dawn (`QueueVk.cpp`) both expose helper-style abstractions with timeouts as first-class arguments, but neither hides `UINT64_MAX` at the type level — `Option<Duration>::None` and `Nanoseconds` newtypes still resolve to a raw `u64`. Vulkan-Hpp, the canonical "type-safe" C++ wrapper, declined to add a `std::chrono::duration` overload for `waitForFences` at all. There is no clang-tidy check, no validation-layer rule, and no Zig-binding precedent for wrapping the dispatch table.
210
211 β + grep-gate matches every production Vulkan project surveyed and avoids being a category of one. The grep gate covers the same gap γ would have prevented, at a fraction of the abstraction cost.
212
213 The one ergonomic borrow from γ is the `waitIdleForShutdown` named helper. This puts intent at the call site without requiring a comment, and replaces what would otherwise be a `// vk-unbounded-ok: shutdown` exemption convention.
214
215 ## Module 7: Grep gate
216
217 A shell script `tests/check_unbounded_vk.sh` runs as part of `zig build test`. It uses `rg --multiline --pcre2` (multi-line is required — `main.zig:1364` proves the existing `waitForFences` call spans 7 lines) to flag any of these identifier patterns outside the exempt files:
218
219 - `\bvkd\.waitForFences\b`
220 - `\bvkd\.acquireNextImageKHR\b`
221 - `\bvkd\.deviceWaitIdle\b`
222 - `\bvkd\.queueWaitIdle\b`
223
224 **Exempt files:**
225
226 - `src/vk_sync.zig` — the helper module itself.
227 - `tests/**/*.zig` — test code may legitimately construct fences/swapchains and call raw `vkd` methods (`tests/vk_sync_test.zig` will need to call `vkd.waitForFences` on never-signaled fences to verify timeout behavior).
228
229 The script exits non-zero if any match is found in non-exempt files, printing each match with file:line. No `// vk-unbounded-ok:` comment exemption — every legitimate unbounded wait goes through `waitIdleForShutdown` (already exempt as the canonical path). If a future case truly needs a raw call, it should add a method to `vk_sync.zig`, not bypass the gate.
230
231 The choice of shell script over a Zig AST walker is deliberate: four identifier patterns, one rg invocation, no parsing complexity, and the implementation is ~10 lines.
232
233 ## Testing
234
235 Three layers:
236
237 **Unit tests (`tests/vk_sync_test.zig`):**
238
239 - `waitFenceBounded` returns `error.VkWaitTimeout` for a never-signaled fence within `fence_wait_timeout_ns + 50ms` slack.
240 - `waitFenceBounded` succeeds for a fence signaled before the call.
241 - `acquireImageBounded` returns the image_index for a healthy swapchain.
242 - `acquireImageBounded` returns `error.VkAcquireTimeout` when all images are in flight (saturate the swapchain by acquiring without submitting).
243 - `acquireImageBounded` returns `error.OutOfDateKHR` for both `VK_SUBOPTIMAL_KHR` and `VK_ERROR_OUT_OF_DATE_KHR` results.
244 - `logVkTimeout` emits exactly one log line when called 100 times in a tight loop (rate-limit working).
245 - `logVkTimeout` emits a second log line if called again after 5+ seconds.
246
247 **Integration test (manual or scripted):** simulate the wedge by submitting a fence that intentionally never signals (e.g., wait on a semaphore that's never signaled), then run a few frames of `drawCells`. Verify: no hang, log lines appear with backoff, main loop continues, Wayland input dispatch keeps working. This is hard to fully automate without a Vulkan mock layer; it can be scripted as a sanity check the human runs by hand.
248
249 **Grep gate:** the test described in Module 7. Runs on every `zig build test`.
250
251 ## Implementation order
252
253 A plan-stage detail, but worth flagging: the reorder fix (Module 3) and the helper introduction (Module 1) should land in separate commits. The reorder is a correctness fix that makes sense in isolation; the helpers stack on top.
254
255 Suggested commit order:
256
257 1. Add `src/vk_sync.zig` with helpers + tests. Wire it into `build.zig` (see "Build wiring" below). No callers changed yet.
258 2. Reorder `resetFences` to after acquire in `drawClear` and `drawCells`. No new errors yet — keep `UINT64_MAX` for one commit. Add the post-acquire `errdefer` re-signal helper.
259 3. Migrate the 10 timeout sites to bounded helpers. Add the timeout error arms in `main.zig`.
260 4. Migrate the 14 `deviceWaitIdle` / `queueWaitIdle` sites to `waitIdleForShutdown`.
261 5. Add the grep gate test.
262 6. Add logging + backoff in `main.zig`.
263
264 ### Build wiring
265
266 `src/vk_sync.zig` is imported by both `src/renderer.zig` and `src/main.zig`. It needs to be a separate module in `build.zig`, similar to how `cell_instance` is wired (build.zig:311). Concretely: create `vk_sync_mod` with `addImport("vulkan", vulkan_module)`; have `renderer_mod`, `exe_mod`, `main_test_mod`, `renderer_test_mod`, and `capture_mod` each `addImport("vk_sync", vk_sync_mod)`. Also add a `vk_sync_tests` step pointing at `tests/vk_sync_test.zig` (with `addImport("vulkan", vulkan_module)` and `addImport("vk_sync", vk_sync_mod)`), and add it to the `test` step alongside the other test invocations.
267
268 The grep gate `tests/check_unbounded_vk.sh` is invoked from `build.zig`'s `test` step via `b.addSystemCommand(&.{ "tests/check_unbounded_vk.sh" })` and added as a step dependency.
269
270 ## Ticket cleanup
271
272 - **Close `793f491a`** as duplicate of `ab6c92f0` (older by ~1 minute, identical body).
273 - **Open follow-up ticket** "Audit `deviceWaitIdle` / `queueWaitIdle` for unbounded blocks" — referenced by this design and seeded with the 15 sites that get mechanically migrated to `waitIdleForShutdown` in Module 2. Each site should be reviewed for whether the unbounded behavior is actually correct (most likely yes for `deinit`; the recovery arm at `main.zig:681` is the most suspicious site).
274 - Move `ab6c92f0` from `backlog` to `planning` after this spec lands; to `dev` when implementation starts.