From c8b5739e1a6c258f74a899dbee0edc7aa072fcc0 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sun, 18 Oct 2026 03:27:50 +0200 Subject: [PATCH] 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. --- implementations/Crafter.Graphics-Window.cpp | 18 ++++++++++-------- interfaces/Crafter.Graphics-Window.cppm | 4 ++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/implementations/Crafter.Graphics-Window.cpp b/implementations/Crafter.Graphics-Window.cpp index 483d9ee..9603065 100644 --- a/implementations/Crafter.Graphics-Window.cpp +++ b/implementations/Crafter.Graphics-Window.cpp @@ -521,7 +521,7 @@ Window::~Window() { // 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) { - 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++) { if (renderFinishedSemaphores[i]) vkDestroySemaphore(Device::device, renderFinishedSemaphores[i], nullptr); if (waitFences[i]) vkDestroyFence(Device::device, waitFences[i], nullptr); @@ -537,9 +537,10 @@ Window::~Window() { #ifdef CRAFTER_GRAPHICS_WINDOW_WAYLAND SetDefaultCursor(); - // `cb` is not destroyed here: wl_surface_frame_done frees the callback it - // was called with and re-arms through its own local, so the member may - // already name a freed one. + // A pending frame callback would still dispatch into this window; + // destroying the proxy makes libwayland drop its event. + if (cb) wl_callback_destroy(cb); + cb = nullptr; if (wpViewport) wp_viewport_destroy(wpViewport); if (wp_scale) { wp_fractional_scale_v1_destroy(wp_scale); @@ -554,7 +555,7 @@ Window::~Window() { xdgToplevel = nullptr; xdgSurface = nullptr; surface = nullptr; - wl_display_flush(Device::display); + if (Device::display) wl_display_flush(Device::display); std::erase(Device::windows, this); if (Device::focusedWindow == this) Device::focusedWindow = 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); Window* window = reinterpret_cast(data); + // The member names the live callback, so the destructor can cancel it. if(window->updating) { - cb = wl_surface_frame(window->surface); - wl_callback_add_listener(cb, &Window::wl_callback_listener, window); + window->cb = wl_surface_frame(window->surface); + wl_callback_add_listener(window->cb, &Window::wl_callback_listener, window); window->Update(); } else { - cb = nullptr; + window->cb = nullptr; } } diff --git a/interfaces/Crafter.Graphics-Window.cppm b/interfaces/Crafter.Graphics-Window.cppm index b4e4176..03a5710 100644 --- a/interfaces/Crafter.Graphics-Window.cppm +++ b/interfaces/Crafter.Graphics-Window.cppm @@ -256,7 +256,7 @@ export namespace Crafter { constexpr static wp_fractional_scale_v1_listener wp_fractional_scale_v1_listener = { .preferred_scale = xdg_surface_handle_preferred_scale, }; - inline static wp_fractional_scale_v1* wp_scale = nullptr; + wp_fractional_scale_v1* wp_scale = nullptr; #endif #ifndef CRAFTER_GRAPHICS_WINDOW_DOM @@ -298,7 +298,7 @@ export namespace Crafter { // lazily in Render() (post-acquire) rather than up front. std::array imageInitialised{}; std::thread thread; - VkCommandBuffer drawCmdBuffers[numFrames]; + VkCommandBuffer drawCmdBuffers[numFrames] = {}; // Per-frame Vulkan info structs whose contents are almost entirely // invariant. Initialised once in the constructor; Render() patches only // the few fields that change each frame (the two barriers' image and