From b07de934ccc8783a48b1055d0207dc06701eeb31 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Thu, 19 Oct 2017 12:03:06 +0300 Subject: [PATCH] compositor-wayland: avoid recursive dispatch with wl_outputs Calling wl_display_roundtrip() from within a Wayland event handler means we will re-enter event dispatch, that is, it will lead to recursive dispatch. Even if libwayland-client was safe, this would lead to unexpected code paths: the first event handler function has not returned when other event handler functions may get called. In this particular case it maybe didn't hurt, but it's still a fragile pattern to use. Replace the wl_display_roundtrip() with a manual sync callback to do the work. This does not break the wayland-backend initialization sequence, because sprawl_across_outputs was set only after the roundtrip to ensure wl_registry globals have been received so the code would not have been hit anyway, and weston_backend_init() also has a second roundtrip that ensures the per wl_output events have been received before continuing. For wayland-backend output hotplug the change is insignificant because it will only delay the output creation a bit, and the parent outputs are not processed anywhere in between. Signed-off-by: Pekka Paalanen Reviewed-by: Quentin Glidic Acked-by: Daniel Stone --- libweston/compositor-wayland.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c index 1f77385b..33910316 100644 --- a/libweston/compositor-wayland.c +++ b/libweston/compositor-wayland.c @@ -136,6 +136,7 @@ struct wayland_output { }; struct wayland_parent_output { + struct wayland_backend *backend; /**< convenience */ struct wayland_output *output; struct wl_list link; @@ -153,6 +154,8 @@ struct wayland_parent_output { uint32_t transform; uint32_t scale; + struct wl_callback *sync_cb; /**< wl_output < 2 done replacement */ + struct wl_list mode_list; struct weston_mode *preferred_mode; struct weston_mode *current_mode; @@ -2258,6 +2261,24 @@ static const struct wl_output_listener output_listener = { wayland_parent_output_mode }; +static void +output_sync_callback(void *data, struct wl_callback *callback, uint32_t unused) +{ + struct wayland_parent_output *output = data; + + assert(output->sync_cb == callback); + wl_callback_destroy(callback); + output->sync_cb = NULL; + + assert(output->backend->sprawl_across_outputs); + + wayland_output_create_for_parent_output(output->backend, output); +} + +static const struct wl_callback_listener output_sync_listener = { + output_sync_callback +}; + static void wayland_backend_register_output(struct wayland_backend *b, uint32_t id) { @@ -2267,6 +2288,7 @@ wayland_backend_register_output(struct wayland_backend *b, uint32_t id) if (!output) return; + output->backend = b; output->id = id; output->global = wl_registry_bind(b->parent.registry, id, &wl_output_interface, 1); @@ -2284,8 +2306,9 @@ wayland_backend_register_output(struct wayland_backend *b, uint32_t id) wl_list_insert(&b->parent.output_list, &output->link); if (b->sprawl_across_outputs) { - wl_display_roundtrip(b->parent.wl_display); - wayland_output_create_for_parent_output(b, output); + output->sync_cb = wl_display_sync(b->parent.wl_display); + wl_callback_add_listener(output->sync_cb, + &output_sync_listener, output); } } @@ -2294,6 +2317,9 @@ wayland_parent_output_destroy(struct wayland_parent_output *output) { struct weston_mode *mode, *next; + if (output->sync_cb) + wl_callback_destroy(output->sync_cb); + if (output->output) wayland_output_destroy(&output->output->base);