Flush a frame buffer's deferred destroy while its window still exists (fixes Vulkan deadlock on window teardown) - #98
Open
owlet-labs wants to merge 1 commit into
Conversation
…xists
Destroying a window-paired layer deadlocks the calling thread on Vulkan.
Reproduced by opening a plugin editor, closing it and opening it again;
in a DAW that wedges the host's main thread, so the editor never reopens
and saving the project hangs.
bgfx::destroy only QUEUES the destruction — the backend performs it
inside a later bgfx::frame(). Layer::removeFromWindow queued it and
returned, and nothing submitted a frame before the window itself was torn
down, so the deferred FrameBufferVK::destroy ran much later, inside the
next createFrameBuffer, when its surface no longer existed.
SwapChainVK::destroy then waits in CommandQueueVK::consume for a queue
that can never drain, because Mesa's WSI thread is parked in
xcb_wait_for_special_event waiting for a Present completion from a
destroyed window. Captured stack:
bgfx::vk::CommandQueueVK::consume()
bgfx::vk::SwapChainVK::destroy()
bgfx::vk::FrameBufferVK::destroy()
bgfx::vk::RendererContextVK::createFrameBuffer(...)
bgfx::Context::frame(bool)
visage::Canvas::submit(int)
visage::ApplicationEditor::addToWindow(visage::Window*)
visage::ApplicationWindow::show(void*)
So the order was wrong rather than the operation. removeFromWindow now
submits two frames after destroying, while the window is still alive, so
the destroy executes against a surface that still exists — the same
idiom post_effects.cpp already uses after destroying its own frame
buffers.
Guarded on there having been a frame buffer at all: show() calls
removeFromWindow before building its FIRST window too, when there is no
renderer yet — bgfx::init runs later in addToWindow — and submitting a
frame there crashes outright. That is not hypothetical; it is what the
unguarded version did.
Verified in a CLAP plugin host harness that opens, closes and reopens the
editor: three cycles clean where every previous attempt hung on the
second, and four clean with the plugin instance destroyed and re-created
between rounds, which is the case a plugin-side workaround cannot reach.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Showing a
ApplicationWindowa second time — or destroying one and creating another in the sameprocess — deadlocks the calling thread on the Vulkan backend. In a plugin this is the host's main
thread, so the editor never reopens, the DAW hangs when it next needs that thread (saving the
project, for us), and the host eventually reports the plugin as not responding.
Reproduction
A CLAP plugin editor, opened and closed and opened again.
ApplicationWindow::show(parent)beginswith
removeFromWindow()and then builds a new window, so the second open is enough:gui->create/set_parent→ editor appears, draws normallygui->destroygui->create/set_parent→ never returnsAlso reachable without reopening: destroy the object that owns the
ApplicationWindowand createanother one (for us, a second plugin instance after the first is removed).
Where it blocks
eu-stackon the wedged process:and on another thread, the other half:
The mechanism
bgfx::destroyonly QUEUES the destruction; the backend performs it inside a laterbgfx::frame().Layer::removeFromWindow()(visage_graphics/layer.h) queues the frame buffer's destroy and returns:Nothing submits a frame between that and the window itself being torn down. So the deferred
FrameBufferVK::destroy()runs much later — inside the NEXTcreateFrameBuffer— by which time itssurface is gone.
SwapChainVK::destroy()then waits inCommandQueueVK::consume()for a queue thatcan never drain, because Mesa's WSI thread is waiting for a Present completion from a window that no
longer exists.
The order is wrong rather than the operation.
The fix we are running
Submit the frames that execute the deferred destroy while the window still exists:
Two frames because that is what it takes for a deferred command to be executed and retired, and it
is the idiom
post_effects.cpp'sdestroyFrameBuffers()already uses after destroying its ownframe buffers — which is part of why we think this is a missing call rather than a design choice.
The guard is not hypothetical: without it the fix crashes on the very first open, because
show()calls
removeFromWindowbefore there is any renderer at all.Verified
In a CLAP host harness that opens, closes and reopens the editor: three cycles clean where every
previous attempt hung on the second, and four cycles clean with the owning object destroyed and
re-created between rounds. Linux/X11, Mesa RADV,
VISAGE_BACKGROUND_GRAPHICS_THREAD=1.Note for maintainers
We could not find a supported way to avoid this from the application side.
Renderer::initializelatches
initialized_and nothing resets it,bgfx::shutdownis called nowhere in the tree, andApplicationWindow::show()always destroys and rebuilds. Our own interim workaround was to keep onewindow for the life of the process and reparent it between host containers with plain X11 — which
works, but cannot cover the object being destroyed, and is not something an application should have
to invent.