Repository navigation
fix(scene): GPU tests exit cleanly on NVIDIA - #79
Conversation
Quest-Claim: 13d7d41c7baba5ed
…anly on NVIDIA The overlay thread was detached and freed Vello's pipelines after the compositor dropped, racing NVIDIA's exit handlers when the process ended: a SIGSEGV or free() abort after each overlay test passed. Overlays now stops the thread and joins it on drop. The matte thread held GPU objects the same way, so Matte joins its thread on drop too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 28 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 85536dd
Base: 2bbdd5b
No actionable findings.
Direction: joining the GPU-owning threads fixes their lifetime rather than hiding the exit race. The explicit overlay Stop message avoids depending on every Overlay sender clone being dropped first; the join also covers a worker already unwinding after a panic. Matte closes its sole job sender before joining, keeps the device/readback resources alive through that join, and its worker polls its own submitted GPU work. Its unbounded result channel does not require the dropping compositor to consume a result before the worker exits. The existing one-picture-at-a-time ownership bounds the work drained during matte shutdown.
Verification: GitHub-only static review of the changes, worker request/result loops, GPU resource ownership, compositor cleanup/failure paths and quest closure. No builds, tests, core-dump inspection, NVIDIA/Metal runs or timing measurements performed. The reported repeated GPU-suite passes and shutdown latency were not independently reproduced. This is a COMMENT, not approval.
(Written by review (OpenAI))

Problem
On NVIDIA under Linux (nixos: RTX 3070 Ti, driver 580.178.04), 7 compositor GPU tests passed and then crashed when the process exited, with SIGSEGV or SIGABRT (
free(): invalid pointer):a_move_glides_an_overlay,an_overlay_*(4),a_stinger_mask_runs_on_under_a_preview, andscaling_bleeds_no_transparent_color. Found in moq-dev/studio#74.Reproduced at main (2bbdd5b): 41/48
compositor::testspass, the same 7 abort;a_move_glides_an_overlayrun alone crashed 6 of 6 times.Root cause, from the core dump: the overlay thread is detached. When the compositor drops, the thread wakes, frees Vello's pipelines (
Painterdrop,vkDestroyDescriptorPoolinlibnvidia-eglcore), while the main thread has already returned and is inexit(), runninglibEGL_nvidia's exit handlers that tear down the driver. The overlay thread then crashes inside the driver. Every failing test opens an overlay; Mesa's lavapipe has no such exit-time teardown, so it never crashed there. Not a wgpu, driver, or moq bug: freeing GPU objects aftermainreturns is the studio's.Approach
Overlayskeeps its thread'sJoinHandle. Dropping it sends a newRequest::Stopand joins the thread, so the documents and the painter are freed before the compositor finishes dropping.Stopis needed because eachOverlayholds a sender clone, so the channel would not close on its own.Mattehas the same pattern: its detached thread holds a device clone and the readback buffer, and frees them after the matte drops. It now drops its job sender and joins the thread on drop. No test hit this one (the matte tests wait for the thread before dropping), but the race is the same.CompletionandWorker, which join their GPU threads on drop.Impact
a_move_glides_an_overlayon the RTX 3070 Ti (debug and release). This happens only at shutdown, or when the overlay thread has already failed.pub(crate)). Wire: none. README unchanged: no flag, panel, scene file, or output change.Alternatives
Stop: depends on everyOverlaydropping beforeOverlays, and a stray one would hang the join forever.Choices made without asking
Browser::stopalready joins it, and moq-dev/studio#76 is working in that module.Validation
On nixos (RTX 3070 Ti, NVIDIA 580.178.04), at 85536dd:
compositor::tests(--run-ignored only -j 4)a_move_glides_an_overlayalone, the test binary directlycompositor::tests+browser::layers::tests, NVIDIAcompositor::tests+browser::layers::tests, lavapipe (VK_DRIVER_FILES)engine::tests::the_program_*,setup::tests::times_every_canvas, NVIDIAjust cicoredumpctlon a crash at main; under gdb the test passed, since gdb's slower teardown hides the race.-j 4because another process holds 5.6 of the GPU's 8 GB; moq-dev/studio#74 hitRequestDeviceError(OutOfMemory)at default parallelism. No out-of-memory errors at-j 4.eprintln!probes, not committed.Follow-ups
None.
Review
quest review), gpt-6.1-sol, medium effort.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code