From c84ac6d6bcd8f9d1e8c16c0aa996653d48b49b08 Mon Sep 17 00:00:00 2001 From: Frank Praznik Date: Sat, 18 Oct 2025 14:16:45 -0400 Subject: [PATCH] wayland: Fix a race condition in the color management event handlers The queue running on the cursor thread might flush color events before their queue has been set. Use proxy wrappers for their parent objects to assign the queue atomically at creation time. --- src/video/wayland/SDL_waylandcolor.c | 54 +++++++++++++++++++++------- src/video/wayland/SDL_waylandmouse.c | 2 +- src/video/wayland/SDL_waylandsym.h | 4 +++ src/video/wayland/SDL_waylandvideo.c | 12 +++++++ src/video/wayland/SDL_waylandvideo.h | 1 + 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/video/wayland/SDL_waylandcolor.c b/src/video/wayland/SDL_waylandcolor.c index dfc69bb5f4..e678bb0444 100644 --- a/src/video/wayland/SDL_waylandcolor.c +++ b/src/video/wayland/SDL_waylandcolor.c @@ -33,6 +33,7 @@ typedef struct Wayland_ColorInfoState { struct wp_image_description_v1 *wp_image_description; struct wp_image_description_info_v1 *wp_image_description_info; + struct wl_event_queue *queue; union { @@ -73,6 +74,9 @@ void Wayland_FreeColorInfoState(Wayland_ColorInfoState *state) { if (state) { Wayland_CancelColorInfoRequest(state); + if (state->queue) { + WAYLAND_wl_event_queue_destroy(state->queue); + } switch (state->object_type) { case WAYLAND_COLOR_OBJECT_TYPE_WINDOW: @@ -212,18 +216,10 @@ static void PumpColorspaceEvents(Wayland_ColorInfoState *state) SDL_VideoData *vid = SDL_GetVideoDevice()->internal; // Run the image description sequence to completion in its own queue. - struct wl_event_queue *queue = WAYLAND_wl_display_create_queue(vid->display); - if (state->deferred_event_processing) { - WAYLAND_wl_proxy_set_queue((struct wl_proxy *)state->wp_image_description_info, queue); - } else { - WAYLAND_wl_proxy_set_queue((struct wl_proxy *)state->wp_image_description, queue); - } - while (state->wp_image_description) { - WAYLAND_wl_display_dispatch_queue(vid->display, queue); + WAYLAND_wl_display_dispatch_queue(vid->display, state->queue); } - WAYLAND_wl_event_queue_destroy(queue); Wayland_FreeColorInfoState(state); } @@ -246,8 +242,20 @@ static void image_description_handle_ready(void *data, { Wayland_ColorInfoState *state = (Wayland_ColorInfoState *)data; - // This will inherit the queue of the factory image description object. - state->wp_image_description_info = wp_image_description_v1_get_information(state->wp_image_description); + /* If event processing was deferred, then the image description is on the default queue. + * Otherwise, it will inherit the queue from the image description object. + */ + if (state->deferred_event_processing) { + SDL_VideoData *vid = SDL_GetVideoDevice()->internal; + state->queue = Wayland_DisplayCreateQueue(vid->display, "SDL Color Management Queue"); + + struct wl_proxy *image_desc_wrapper = WAYLAND_wl_proxy_create_wrapper(state->wp_image_description); + WAYLAND_wl_proxy_set_queue(image_desc_wrapper, state->queue); + state->wp_image_description_info = wp_image_description_v1_get_information((struct wp_image_description_v1 *)image_desc_wrapper); + WAYLAND_wl_proxy_wrapper_destroy(image_desc_wrapper); + } else { + state->wp_image_description_info = wp_image_description_v1_get_information(state->wp_image_description); + } wp_image_description_info_v1_add_listener(state->wp_image_description_info, &image_description_info_listener, data); if (state->deferred_event_processing) { @@ -271,7 +279,17 @@ void Wayland_GetColorInfoForWindow(SDL_WindowData *window_data, bool defer_event state->window_data = window_data; state->object_type = WAYLAND_COLOR_OBJECT_TYPE_WINDOW; state->deferred_event_processing = defer_event_processing; - state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred(window_data->wp_color_management_surface_feedback); + + if (!defer_event_processing) { + state->queue = Wayland_DisplayCreateQueue(window_data->waylandData->display, "SDL Color Management Queue"); + + struct wl_proxy *surface_feedback_wrapper = WAYLAND_wl_proxy_create_wrapper(window_data->wp_color_management_surface_feedback); + WAYLAND_wl_proxy_set_queue(surface_feedback_wrapper, state->queue); + state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred((struct wp_color_management_surface_feedback_v1 *)surface_feedback_wrapper); + WAYLAND_wl_proxy_wrapper_destroy(surface_feedback_wrapper); + } else { + state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred(window_data->wp_color_management_surface_feedback); + } wp_image_description_v1_add_listener(state->wp_image_description, &image_description_listener, state); if (!defer_event_processing) { @@ -291,7 +309,17 @@ void Wayland_GetColorInfoForOutput(SDL_DisplayData *display_data, bool defer_eve state->display_data = display_data; state->object_type = WAYLAND_COLOR_OBJECT_TYPE_DISPLAY; state->deferred_event_processing = defer_event_processing; - state->wp_image_description = wp_color_management_output_v1_get_image_description(display_data->wp_color_management_output); + + if (!defer_event_processing) { + state->queue = Wayland_DisplayCreateQueue(display_data->videodata->display, "SDL Color Management Queue"); + + struct wl_proxy *output_feedback_wrapper = WAYLAND_wl_proxy_create_wrapper(display_data->wp_color_management_output); + WAYLAND_wl_proxy_set_queue(output_feedback_wrapper, state->queue); + state->wp_image_description = wp_color_management_output_v1_get_image_description((struct wp_color_management_output_v1 *)output_feedback_wrapper); + WAYLAND_wl_proxy_wrapper_destroy(output_feedback_wrapper); + } else { + state->wp_image_description = wp_color_management_output_v1_get_image_description(display_data->wp_color_management_output); + } wp_image_description_v1_add_listener(state->wp_image_description, &image_description_listener, state); if (!defer_event_processing) { diff --git a/src/video/wayland/SDL_waylandmouse.c b/src/video/wayland/SDL_waylandmouse.c index eedfc5d9e8..19d53ab1ca 100644 --- a/src/video/wayland/SDL_waylandmouse.c +++ b/src/video/wayland/SDL_waylandmouse.c @@ -390,7 +390,7 @@ static int SDLCALL Wayland_CursorThreadFunc(void *data) static bool Wayland_StartCursorThread(SDL_VideoData *data) { if (!cursor_thread_context.thread) { - cursor_thread_context.queue = WAYLAND_wl_display_create_queue(data->display); + cursor_thread_context.queue = Wayland_DisplayCreateQueue(data->display, "SDL Cursor Surface Queue"); if (!cursor_thread_context.queue) { goto cleanup; } diff --git a/src/video/wayland/SDL_waylandsym.h b/src/video/wayland/SDL_waylandsym.h index 8d45d59ffd..a1e9dab7fd 100644 --- a/src/video/wayland/SDL_waylandsym.h +++ b/src/video/wayland/SDL_waylandsym.h @@ -88,6 +88,10 @@ SDL_WAYLAND_SYM(struct wl_proxy *, wl_proxy_marshal_flags, (struct wl_proxy *pro SDL_WAYLAND_SYM(struct wl_proxy *, wl_proxy_marshal_array_flags, (struct wl_proxy *proxy, uint32_t opcode, const struct wl_interface *interface, uint32_t version, uint32_t flags, union wl_argument *args)) #endif +#if SDL_WAYLAND_CHECK_VERSION(1, 23, 0) || defined(SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC) +SDL_WAYLAND_SYM_OPT(struct wl_event_queue *, wl_display_create_queue_with_name, (struct wl_display *display, const char *name)) +#endif + #if 0 // TODO RECONNECT: See waylandvideo.c for more information! #if SDL_WAYLAND_CHECK_VERSION(broken, on, purpose) SDL_WAYLAND_SYM(int, wl_display_reconnect, (struct wl_display *)) diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c index 4d37c497bb..e266c5f2fd 100644 --- a/src/video/wayland/SDL_waylandvideo.c +++ b/src/video/wayland/SDL_waylandvideo.c @@ -457,6 +457,18 @@ SDL_WindowData *Wayland_GetWindowDataForOwnedSurface(struct wl_surface *surface) return NULL; } +struct wl_event_queue *Wayland_DisplayCreateQueue(struct wl_display *display, const char *name) +{ +#ifdef SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC + if (WAYLAND_wl_display_create_queue_with_name) { + return WAYLAND_wl_display_create_queue_with_name(display, name); + } +#elif SDL_WAYLAND_CHECK_VERSION(1, 23, 0) + return WAYLAND_wl_display_create_queue_with_name(display, name); +#endif + return WAYLAND_wl_display_create_queue(display); +} + static void Wayland_DeleteDevice(SDL_VideoDevice *device) { SDL_VideoData *data = device->internal; diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h index 644264a4a8..72e9c0718d 100644 --- a/src/video/wayland/SDL_waylandvideo.h +++ b/src/video/wayland/SDL_waylandvideo.h @@ -137,6 +137,7 @@ extern bool SDL_WAYLAND_own_output(struct wl_output *output); extern SDL_WindowData *Wayland_GetWindowDataForOwnedSurface(struct wl_surface *surface); void Wayland_AddWindowDataToExternalList(SDL_WindowData *data); void Wayland_RemoveWindowDataFromExternalList(SDL_WindowData *data); +struct wl_event_queue *Wayland_DisplayCreateQueue(struct wl_display *display, const char *name); extern bool Wayland_LoadLibdecor(SDL_VideoData *data, bool ignore_xdg);