fix(window): the destructor cancels the frame callback; a window without a display or swapchain

wl_surface_frame_done re-armed the frame callback through its local and
left the member stale, so the destructor could not cancel it and a done
already queued would dispatch into the freed window; the member now
names the live callback and the destructor destroys it. The destructor
also flushed a null display for a window created without
Device::Initialize (tests do that) and freed never-created command
buffers (a default-constructed Window after Initialize); both guarded.
And wp_scale was one inline static for every window, which the destructor
destroyed on behalf of all of them: it is a member now, like the viewport.
This commit is contained in:
Jorijn van der Graaf 2026-10-18 03:27:50 +02:00
commit c8b5739e1a
Signed by: jorijnvdgraaf
GPG key ID: 2937E59CDCC1BCFB
2 changed files with 12 additions and 10 deletions

View file

@ -521,7 +521,7 @@ Window::~Window() {
// Nothing may still run against the semaphores, fences or images below. // Nothing may still run against the semaphores, fences or images below.
if (Device::device != VK_NULL_HANDLE) vkDeviceWaitIdle(Device::device); if (Device::device != VK_NULL_HANDLE) vkDeviceWaitIdle(Device::device);
if (Device::device != VK_NULL_HANDLE) { if (Device::device != VK_NULL_HANDLE) {
vkFreeCommandBuffers(Device::device, Device::commandPool, numFrames, drawCmdBuffers); if (drawCmdBuffers[0] != VK_NULL_HANDLE) vkFreeCommandBuffers(Device::device, Device::commandPool, numFrames, drawCmdBuffers);
for (std::uint8_t i = 0; i < numFrames; i++) { for (std::uint8_t i = 0; i < numFrames; i++) {
if (renderFinishedSemaphores[i]) vkDestroySemaphore(Device::device, renderFinishedSemaphores[i], nullptr); if (renderFinishedSemaphores[i]) vkDestroySemaphore(Device::device, renderFinishedSemaphores[i], nullptr);
if (waitFences[i]) vkDestroyFence(Device::device, waitFences[i], nullptr); if (waitFences[i]) vkDestroyFence(Device::device, waitFences[i], nullptr);
@ -537,9 +537,10 @@ Window::~Window() {
#ifdef CRAFTER_GRAPHICS_WINDOW_WAYLAND #ifdef CRAFTER_GRAPHICS_WINDOW_WAYLAND
SetDefaultCursor(); SetDefaultCursor();
// `cb` is not destroyed here: wl_surface_frame_done frees the callback it // A pending frame callback would still dispatch into this window;
// was called with and re-arms through its own local, so the member may // destroying the proxy makes libwayland drop its event.
// already name a freed one. if (cb) wl_callback_destroy(cb);
cb = nullptr;
if (wpViewport) wp_viewport_destroy(wpViewport); if (wpViewport) wp_viewport_destroy(wpViewport);
if (wp_scale) { if (wp_scale) {
wp_fractional_scale_v1_destroy(wp_scale); wp_fractional_scale_v1_destroy(wp_scale);
@ -554,7 +555,7 @@ Window::~Window() {
xdgToplevel = nullptr; xdgToplevel = nullptr;
xdgSurface = nullptr; xdgSurface = nullptr;
surface = nullptr; surface = nullptr;
wl_display_flush(Device::display); if (Device::display) wl_display_flush(Device::display);
std::erase(Device::windows, this); std::erase(Device::windows, this);
if (Device::focusedWindow == this) Device::focusedWindow = nullptr; if (Device::focusedWindow == this) Device::focusedWindow = nullptr;
if (Device::keyboardWindow == this) Device::keyboardWindow = nullptr; if (Device::keyboardWindow == this) Device::keyboardWindow = nullptr;
@ -1299,12 +1300,13 @@ void Window::wl_surface_frame_done(void* data, struct wl_callback *cb, uint32_t
wl_callback_destroy(cb); wl_callback_destroy(cb);
Window* window = reinterpret_cast<Window*>(data); Window* window = reinterpret_cast<Window*>(data);
// The member names the live callback, so the destructor can cancel it.
if(window->updating) { if(window->updating) {
cb = wl_surface_frame(window->surface); window->cb = wl_surface_frame(window->surface);
wl_callback_add_listener(cb, &Window::wl_callback_listener, window); wl_callback_add_listener(window->cb, &Window::wl_callback_listener, window);
window->Update(); window->Update();
} else { } else {
cb = nullptr; window->cb = nullptr;
} }
} }

View file

@ -256,7 +256,7 @@ export namespace Crafter {
constexpr static wp_fractional_scale_v1_listener wp_fractional_scale_v1_listener = { constexpr static wp_fractional_scale_v1_listener wp_fractional_scale_v1_listener = {
.preferred_scale = xdg_surface_handle_preferred_scale, .preferred_scale = xdg_surface_handle_preferred_scale,
}; };
inline static wp_fractional_scale_v1* wp_scale = nullptr; wp_fractional_scale_v1* wp_scale = nullptr;
#endif #endif
#ifndef CRAFTER_GRAPHICS_WINDOW_DOM #ifndef CRAFTER_GRAPHICS_WINDOW_DOM
@ -298,7 +298,7 @@ export namespace Crafter {
// lazily in Render() (post-acquire) rather than up front. // lazily in Render() (post-acquire) rather than up front.
std::array<bool, numFrames> imageInitialised{}; std::array<bool, numFrames> imageInitialised{};
std::thread thread; std::thread thread;
VkCommandBuffer drawCmdBuffers[numFrames]; VkCommandBuffer drawCmdBuffers[numFrames] = {};
// Per-frame Vulkan info structs whose contents are almost entirely // Per-frame Vulkan info structs whose contents are almost entirely
// invariant. Initialised once in the constructor; Render() patches only // invariant. Initialised once in the constructor; Render() patches only
// the few fields that change each frame (the two barriers' image and // the few fields that change each frame (the two barriers' image and