Skip to content

Commit c84ac6d

Browse files
committed
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.
1 parent 35cc58e commit c84ac6d

File tree

5 files changed

+59
-14
lines changed

5 files changed

+59
-14
lines changed

src/video/wayland/SDL_waylandcolor.c

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ typedef struct Wayland_ColorInfoState
3333
{
3434
struct wp_image_description_v1 *wp_image_description;
3535
struct wp_image_description_info_v1 *wp_image_description_info;
36+
struct wl_event_queue *queue;
3637

3738
union
3839
{
@@ -73,6 +74,9 @@ void Wayland_FreeColorInfoState(Wayland_ColorInfoState *state)
7374
{
7475
if (state) {
7576
Wayland_CancelColorInfoRequest(state);
77+
if (state->queue) {
78+
WAYLAND_wl_event_queue_destroy(state->queue);
79+
}
7680

7781
switch (state->object_type) {
7882
case WAYLAND_COLOR_OBJECT_TYPE_WINDOW:
@@ -212,18 +216,10 @@ static void PumpColorspaceEvents(Wayland_ColorInfoState *state)
212216
SDL_VideoData *vid = SDL_GetVideoDevice()->internal;
213217

214218
// Run the image description sequence to completion in its own queue.
215-
struct wl_event_queue *queue = WAYLAND_wl_display_create_queue(vid->display);
216-
if (state->deferred_event_processing) {
217-
WAYLAND_wl_proxy_set_queue((struct wl_proxy *)state->wp_image_description_info, queue);
218-
} else {
219-
WAYLAND_wl_proxy_set_queue((struct wl_proxy *)state->wp_image_description, queue);
220-
}
221-
222219
while (state->wp_image_description) {
223-
WAYLAND_wl_display_dispatch_queue(vid->display, queue);
220+
WAYLAND_wl_display_dispatch_queue(vid->display, state->queue);
224221
}
225222

226-
WAYLAND_wl_event_queue_destroy(queue);
227223
Wayland_FreeColorInfoState(state);
228224
}
229225

@@ -246,8 +242,20 @@ static void image_description_handle_ready(void *data,
246242
{
247243
Wayland_ColorInfoState *state = (Wayland_ColorInfoState *)data;
248244

249-
// This will inherit the queue of the factory image description object.
250-
state->wp_image_description_info = wp_image_description_v1_get_information(state->wp_image_description);
245+
/* If event processing was deferred, then the image description is on the default queue.
246+
* Otherwise, it will inherit the queue from the image description object.
247+
*/
248+
if (state->deferred_event_processing) {
249+
SDL_VideoData *vid = SDL_GetVideoDevice()->internal;
250+
state->queue = Wayland_DisplayCreateQueue(vid->display, "SDL Color Management Queue");
251+
252+
struct wl_proxy *image_desc_wrapper = WAYLAND_wl_proxy_create_wrapper(state->wp_image_description);
253+
WAYLAND_wl_proxy_set_queue(image_desc_wrapper, state->queue);
254+
state->wp_image_description_info = wp_image_description_v1_get_information((struct wp_image_description_v1 *)image_desc_wrapper);
255+
WAYLAND_wl_proxy_wrapper_destroy(image_desc_wrapper);
256+
} else {
257+
state->wp_image_description_info = wp_image_description_v1_get_information(state->wp_image_description);
258+
}
251259
wp_image_description_info_v1_add_listener(state->wp_image_description_info, &image_description_info_listener, data);
252260

253261
if (state->deferred_event_processing) {
@@ -271,7 +279,17 @@ void Wayland_GetColorInfoForWindow(SDL_WindowData *window_data, bool defer_event
271279
state->window_data = window_data;
272280
state->object_type = WAYLAND_COLOR_OBJECT_TYPE_WINDOW;
273281
state->deferred_event_processing = defer_event_processing;
274-
state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred(window_data->wp_color_management_surface_feedback);
282+
283+
if (!defer_event_processing) {
284+
state->queue = Wayland_DisplayCreateQueue(window_data->waylandData->display, "SDL Color Management Queue");
285+
286+
struct wl_proxy *surface_feedback_wrapper = WAYLAND_wl_proxy_create_wrapper(window_data->wp_color_management_surface_feedback);
287+
WAYLAND_wl_proxy_set_queue(surface_feedback_wrapper, state->queue);
288+
state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred((struct wp_color_management_surface_feedback_v1 *)surface_feedback_wrapper);
289+
WAYLAND_wl_proxy_wrapper_destroy(surface_feedback_wrapper);
290+
} else {
291+
state->wp_image_description = wp_color_management_surface_feedback_v1_get_preferred(window_data->wp_color_management_surface_feedback);
292+
}
275293
wp_image_description_v1_add_listener(state->wp_image_description, &image_description_listener, state);
276294

277295
if (!defer_event_processing) {
@@ -291,7 +309,17 @@ void Wayland_GetColorInfoForOutput(SDL_DisplayData *display_data, bool defer_eve
291309
state->display_data = display_data;
292310
state->object_type = WAYLAND_COLOR_OBJECT_TYPE_DISPLAY;
293311
state->deferred_event_processing = defer_event_processing;
294-
state->wp_image_description = wp_color_management_output_v1_get_image_description(display_data->wp_color_management_output);
312+
313+
if (!defer_event_processing) {
314+
state->queue = Wayland_DisplayCreateQueue(display_data->videodata->display, "SDL Color Management Queue");
315+
316+
struct wl_proxy *output_feedback_wrapper = WAYLAND_wl_proxy_create_wrapper(display_data->wp_color_management_output);
317+
WAYLAND_wl_proxy_set_queue(output_feedback_wrapper, state->queue);
318+
state->wp_image_description = wp_color_management_output_v1_get_image_description((struct wp_color_management_output_v1 *)output_feedback_wrapper);
319+
WAYLAND_wl_proxy_wrapper_destroy(output_feedback_wrapper);
320+
} else {
321+
state->wp_image_description = wp_color_management_output_v1_get_image_description(display_data->wp_color_management_output);
322+
}
295323
wp_image_description_v1_add_listener(state->wp_image_description, &image_description_listener, state);
296324

297325
if (!defer_event_processing) {

src/video/wayland/SDL_waylandmouse.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ static int SDLCALL Wayland_CursorThreadFunc(void *data)
390390
static bool Wayland_StartCursorThread(SDL_VideoData *data)
391391
{
392392
if (!cursor_thread_context.thread) {
393-
cursor_thread_context.queue = WAYLAND_wl_display_create_queue(data->display);
393+
cursor_thread_context.queue = Wayland_DisplayCreateQueue(data->display, "SDL Cursor Surface Queue");
394394
if (!cursor_thread_context.queue) {
395395
goto cleanup;
396396
}

src/video/wayland/SDL_waylandsym.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ SDL_WAYLAND_SYM(struct wl_proxy *, wl_proxy_marshal_flags, (struct wl_proxy *pro
8888
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))
8989
#endif
9090

91+
#if SDL_WAYLAND_CHECK_VERSION(1, 23, 0) || defined(SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC)
92+
SDL_WAYLAND_SYM_OPT(struct wl_event_queue *, wl_display_create_queue_with_name, (struct wl_display *display, const char *name))
93+
#endif
94+
9195
#if 0 // TODO RECONNECT: See waylandvideo.c for more information!
9296
#if SDL_WAYLAND_CHECK_VERSION(broken, on, purpose)
9397
SDL_WAYLAND_SYM(int, wl_display_reconnect, (struct wl_display *))

src/video/wayland/SDL_waylandvideo.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,18 @@ SDL_WindowData *Wayland_GetWindowDataForOwnedSurface(struct wl_surface *surface)
457457
return NULL;
458458
}
459459

460+
struct wl_event_queue *Wayland_DisplayCreateQueue(struct wl_display *display, const char *name)
461+
{
462+
#ifdef SDL_VIDEO_DRIVER_WAYLAND_DYNAMIC
463+
if (WAYLAND_wl_display_create_queue_with_name) {
464+
return WAYLAND_wl_display_create_queue_with_name(display, name);
465+
}
466+
#elif SDL_WAYLAND_CHECK_VERSION(1, 23, 0)
467+
return WAYLAND_wl_display_create_queue_with_name(display, name);
468+
#endif
469+
return WAYLAND_wl_display_create_queue(display);
470+
}
471+
460472
static void Wayland_DeleteDevice(SDL_VideoDevice *device)
461473
{
462474
SDL_VideoData *data = device->internal;

src/video/wayland/SDL_waylandvideo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ extern bool SDL_WAYLAND_own_output(struct wl_output *output);
137137
extern SDL_WindowData *Wayland_GetWindowDataForOwnedSurface(struct wl_surface *surface);
138138
void Wayland_AddWindowDataToExternalList(SDL_WindowData *data);
139139
void Wayland_RemoveWindowDataFromExternalList(SDL_WindowData *data);
140+
struct wl_event_queue *Wayland_DisplayCreateQueue(struct wl_display *display, const char *name);
140141

141142
extern bool Wayland_LoadLibdecor(SDL_VideoData *data, bool ignore_xdg);
142143

0 commit comments

Comments
 (0)