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 <pekka.paalanen@collabora.co.uk>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Acked-by: Daniel Stone <daniels@collabora.com>

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 1f77385..3391031 100644 (file)
--- 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;
@@ -2259,6 +2262,24 @@ static const struct wl_output_listener output_listener = {
 };
 
 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)
 {
        struct wayland_parent_output *output;
@@ -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);