diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 513b8fd8..11f0de36 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -3335,12 +3335,20 @@ - + This global fixes problems with other core-protocol interfaces that cannot be fixed in these interfaces themselves. + + + These errors can be emitted in response to wl_fixes requests. + + + + @@ -3360,6 +3368,36 @@ + + + + Acknowledge the removal of the specified global. + + If no global with the specified name exists or the global is not removed, + the wl_fixes.invalid_ack_remove protocol error will be posted. + + Due to the Wayland protocol being asynchronous, the wl_global objects + cannot be destroyed immediately. For example, if a wl_global is removed + and a client attempts to bind that global around same time, it can + result in a protocol error due to an unknown global name in the bind + request. + + In order to avoid crashing clients, the compositor should remove the + wl_global once it is guaranteed that no more bind requests will come. + + The wl_fixes.ack_global_remove() request is used to signal to the + compositor that the client will not bind the given global anymore. After + all clients acknowledge the removal of the global, the compositor can + safely destroy it. + + The client must call the wl_fixes.ack_global_remove() request in + response to a wl_registry.global_remove() event even if it did not bind + the corresponding global. + + + + diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h index c8a64772..cc08de58 100644 --- a/src/wayland-server-core.h +++ b/src/wayland-server-core.h @@ -222,10 +222,13 @@ wl_display_set_default_max_buffer_size(struct wl_display *display, size_t max_buffer_size); struct wl_client; +struct wl_global; typedef void (*wl_global_bind_func_t)(struct wl_client *client, void *data, uint32_t version, uint32_t id); +typedef void (*wl_global_withdrawn_func_t)(struct wl_global *global); + uint32_t wl_display_get_serial(const struct wl_display *display); @@ -253,6 +256,10 @@ wl_global_create(struct wl_display *display, void wl_global_remove(struct wl_global *global); +void +wl_global_set_withdrawn_listener(struct wl_global *global, + wl_global_withdrawn_func_t func); + void wl_global_destroy(struct wl_global *global); @@ -725,6 +732,11 @@ wl_display_add_protocol_logger(struct wl_display *display, void wl_protocol_logger_destroy(struct wl_protocol_logger *logger); +void +wl_fixes_handle_ack_global_remove(struct wl_resource *fixes_resource, + struct wl_resource *registry_resource, + uint32_t global_name); + #ifdef __cplusplus } #endif diff --git a/src/wayland-server.c b/src/wayland-server.c index 31cc9824..932ac662 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -115,17 +115,31 @@ struct wl_display { size_t max_buffer_size; }; +struct wl_registry { + struct wl_display *display; + struct wl_list offer_list; +}; + struct wl_global { struct wl_display *display; const struct wl_interface *interface; + struct wl_list offer_list; uint32_t name; uint32_t version; void *data; wl_global_bind_func_t bind; + wl_global_withdrawn_func_t withdrawn; struct wl_list link; bool removed; }; +struct wl_global_offer { + struct wl_list global_link; /**< in struct wl_global::offer_list */ + struct wl_list registry_link; /**< in struct wl_registry::offer_list */ + struct wl_resource *registry_resource; + struct wl_global *global; +}; + struct wl_resource { struct wl_object object; wl_resource_destroy_func_t destroy; @@ -1048,7 +1062,8 @@ registry_bind(struct wl_client *client, const char *interface, uint32_t version, uint32_t id) { struct wl_global *global; - struct wl_display *display = resource->data; + struct wl_registry *registry = resource->data; + struct wl_display *display = registry->display; wl_list_for_each(global, &display->global_list, link) if (global->name == name) @@ -1100,9 +1115,76 @@ display_sync(struct wl_client *client, } static void -unbind_resource(struct wl_resource *resource) +wl_global_announce(struct wl_global *global, + struct wl_resource *registry_resource) { + struct wl_global_offer *offer; + struct wl_registry *registry = registry_resource->data; + + offer = zalloc(sizeof *offer); + if (offer == NULL) { + wl_resource_post_no_memory(registry_resource); + return; + } + + offer->registry_resource = registry_resource; + offer->global = global; + wl_list_insert(&global->offer_list, &offer->global_link); + wl_list_insert(®istry->offer_list, &offer->registry_link); + + wl_resource_post_event(registry_resource, + WL_REGISTRY_GLOBAL, + global->name, + global->interface->name, + global->version); +} + +static void +wl_global_offer_destroy(struct wl_global_offer *offer) +{ + wl_list_remove(&offer->global_link); + wl_list_remove(&offer->registry_link); + free(offer); +} + +static void +wl_global_offer_done(struct wl_global_offer *offer) +{ + struct wl_global *global = offer->global; + + wl_global_offer_destroy(offer); + + if (global->removed && global->withdrawn && wl_list_empty(&global->offer_list)) + global->withdrawn(global); +} + +static struct wl_global_offer * +wl_registry_find_offer(struct wl_registry *registry, + uint32_t global_name) +{ + struct wl_global_offer *offer; + + wl_list_for_each(offer, ®istry->offer_list, registry_link) { + if (offer->global->name == global_name) + return offer; + } + + return NULL; +} + +static void +unbind_registry_resource(struct wl_resource *resource) +{ + struct wl_registry *registry = resource->data; + struct wl_global_offer *offer; + + while (!wl_list_empty(®istry->offer_list)) { + offer = wl_container_of(registry->offer_list.next, offer, registry_link); + wl_global_offer_done(offer); + } + wl_list_remove(&resource->link); + free(registry); } static void @@ -1112,6 +1194,7 @@ display_get_registry(struct wl_client *client, struct wl_display *display = resource->data; struct wl_resource *registry_resource; struct wl_global *global; + struct wl_registry *registry; registry_resource = wl_resource_create(client, &wl_registry_interface, 1, id); @@ -1120,20 +1203,25 @@ display_get_registry(struct wl_client *client, return; } + registry = zalloc(sizeof *registry); + if (registry == NULL) { + wl_client_post_no_memory(client); + return; + } + + registry->display = display; + wl_list_init(®istry->offer_list); + wl_resource_set_implementation(registry_resource, ®istry_interface, - display, unbind_resource); + registry, unbind_registry_resource); wl_list_insert(&display->registry_resource_list, ®istry_resource->link); wl_list_for_each(global, &display->global_list, link) if (wl_global_is_visible(client, global) && !global->removed) - wl_resource_post_event(registry_resource, - WL_REGISTRY_GLOBAL, - global->name, - global->interface->name, - global->version); + wl_global_announce(global, registry_resource); } static const struct wl_display_interface display_interface = { @@ -1399,52 +1487,24 @@ wl_global_create(struct wl_display *display, global->version = version; global->data = data; global->bind = bind; + global->withdrawn = NULL; global->removed = false; wl_list_insert(display->global_list.prev, &global->link); + wl_list_init(&global->offer_list); wl_list_for_each(resource, &display->registry_resource_list, link) if (wl_global_is_visible(resource->client, global)) - wl_resource_post_event(resource, - WL_REGISTRY_GLOBAL, - global->name, - global->interface->name, - global->version); + wl_global_announce(global, resource); return global; } -/** Remove the global - * - * \param global The Wayland global. - * - * Broadcast a global remove event to all clients without destroying the - * global. This function can only be called once per global. - * - * wl_global_destroy() removes the global and immediately destroys it. On - * the other end, this function only removes the global, allowing clients - * that have not yet received the global remove event to continue to bind to - * it. - * - * This can be used by compositors to mitigate clients being disconnected - * because a global has been added and removed too quickly. Compositors can call - * wl_global_remove(), then wait an implementation-defined amount of time, then - * call wl_global_destroy(). Note that the destruction of a global is still - * racy, since clients have no way to acknowledge that they received the remove - * event. - * - * \since 1.17.90 - */ -WL_EXPORT void -wl_global_remove(struct wl_global *global) +static void +wl_global_send_removed(struct wl_global *global) { struct wl_display *display = global->display; struct wl_resource *resource; - if (global->removed) - wl_abort("wl_global_remove: called twice on the same " - "global '%s#%"PRIu32"'", global->interface->name, - global->name); - wl_list_for_each(resource, &display->registry_resource_list, link) if (wl_global_is_visible(resource->client, global)) wl_resource_post_event(resource, WL_REGISTRY_GLOBAL_REMOVE, @@ -1453,12 +1513,100 @@ wl_global_remove(struct wl_global *global) global->removed = true; } +/** Remove the global + * + * \param global The Wayland global. + * + * Broadcast a global remove event to all clients without destroying the + * global. This function can only be called once per global. + * + * This can be used by compositors to mitigate clients being disconnected + * because a global has been added and removed too quickly. + * + * Due to the Wayland protocol being asynchronous, the wl_global objects + * cannot be destroyed immediately. For example, if a wl_global is destroyed + * and a client attempts to bind that global around same time, it can + * result in a protocol error due to an unknown global name in the bind request. + * + * In order to avoid crashing clients, the compositor should destroy the + * wl_global once it is guaranteed that no more bind requests will come. + * + * The recommended way to destroy globals is as follows: + * + * - the compositor registers a callback using the wl_global_set_withdrawn_listener() + * function, which will be called when it is safe to call wl_global_destroy(); + * - the compositor calls wl_global_remove(). This will broadcast + * wl_registry.global_remove events to all clients; + * - upon receiving a wl_registry.global_remove event, a client must + * reply to it by calling the wl_fixes.ack_global_remove() request. The + * client must call the wl_fixes.ack_global_remove() request even if it + * did not bind the global; + * - the compositor will call the wl_fixes_handle_ack_global_remove() function when + * it receives the wl_fixes.ack_global_remove() request from the client; + * - after all clients have acknowledged the global removal, the "withdrawn" + * callback will be called to notify the compositor that the + * wl_global_destroy() function can be called now. + * + * \sa wl_global_set_withdrawn_listener + * \sa wl_fixes_handle_ack_global_remove + * \since 1.17.90 + */ +WL_EXPORT void +wl_global_remove(struct wl_global *global) +{ + if (global->removed) + wl_abort("wl_global_remove: called twice on the same " + "global '%s#%"PRIu32"'", global->interface->name, + global->name); + + wl_global_send_removed(global); + + if (global->withdrawn && wl_list_empty(&global->offer_list)) + global->withdrawn(global); +} + +/** Set the withdrawn callback. + * + * \param global The Wayland global. + * \param func The withdrawn callback. + * + * The withdrawn callback notifies the compositor that the global can be + * safely destroyed by calling wl_global_destroy(). The callback will be called + * in the following cases: + * + * - immediately by wl_global_remove(), if there are no registeries where the + * global was announced; + * - or some time later after wl_global_remove(), due to either receiving the + * last wl_fixes_handle_ack_global_remove() or getting the last wl_registry + * where the global was announced destroyed. + * + * The withdrawn callback will never be called without prior wl_global_remove(). + * + * \sa wl_fixes_handle_ack_global_remove + * \since 1.26 + */ +WL_EXPORT void +wl_global_set_withdrawn_listener(struct wl_global *global, + wl_global_withdrawn_func_t func) +{ + global->withdrawn = func; +} + WL_EXPORT void wl_global_destroy(struct wl_global *global) { + struct wl_global_offer *offer; + if (!global->removed) - wl_global_remove(global); + wl_global_send_removed(global); + wl_list_remove(&global->link); + + while (!wl_list_empty(&global->offer_list)) { + offer = wl_container_of(global->offer_list.next, offer, global_link); + wl_global_offer_destroy(offer); + } + free(global); } @@ -2709,6 +2857,47 @@ wl_display_remove_global(struct wl_display *display, struct wl_global *global) wl_global_destroy(global); } +/** Acknowledge global removal by a client + * + * \param fixes_resource The wl_fixes resource. + * \param registry_resource The wl_registry resource. + * \param global_name The unique id of the global. + * + * The compositor should call this function from the wl_fixes.ack_global_remove + * request implementation. + * + * libwayland-server automatically takes care of the implied ack-remove due + * to client disconnection or explicit wl_resource_destroy() of the wl_registry + * resource. + * + * \sa wl_global_remove + * \sa wl_global_set_withdrawn_listener + * \since 1.26 + */ +WL_EXPORT void +wl_fixes_handle_ack_global_remove(struct wl_resource *fixes_resource, + struct wl_resource *registry_resource, + uint32_t global_name) +{ + struct wl_global_offer *offer; + struct wl_registry *registry = registry_resource->data; + + offer = wl_registry_find_offer(registry, global_name); + if (!offer) { + wl_resource_post_error(fixes_resource, WL_FIXES_ERROR_INVALID_ACK_REMOVE, + "the given registry did not announce global %u", global_name); + return; + } + + if (!offer->global->removed) { + wl_resource_post_error(fixes_resource, WL_FIXES_ERROR_INVALID_ACK_REMOVE, + "global %u is not removed", global_name); + return; + } + + wl_global_offer_done(offer); +} + /** \endcond */ /* Functions at the end of this file are deprecated. Instead of adding new diff --git a/tests/display-test.c b/tests/display-test.c index fe78b521..e6efa41e 100644 --- a/tests/display-test.c +++ b/tests/display-test.c @@ -51,6 +51,63 @@ #include "tests-server-protocol.h" #include "tests-client-protocol.h" +static void +registry_handle_global_noop(void *data, + struct wl_registry *registry, + uint32_t id, const char *interface, + uint32_t ver) +{ +} + +static void +registry_handle_global_remove_noop(void *data, + struct wl_registry *registry, + uint32_t id) +{ +} + +static void +fixes_destroy(struct wl_client *client, + struct wl_resource *resource) +{ + wl_resource_destroy(resource); +} + +static void +fixes_destroy_registry(struct wl_client *client, + struct wl_resource *resource, + struct wl_resource *registry) +{ + wl_resource_destroy(registry); +} + +static void +fixes_ack_global_remove(struct wl_client *client, + struct wl_resource *resource, + struct wl_resource *registry, + uint32_t global_id) +{ + wl_fixes_handle_ack_global_remove(resource, registry, global_id); +} + +static const struct wl_fixes_interface fixes_implementation = { + .destroy = fixes_destroy, + .destroy_registry = fixes_destroy_registry, + .ack_global_remove = fixes_ack_global_remove, +}; + +static void +bind_fixes(struct wl_client *client, + void *data, + uint32_t version, + uint32_t id) +{ + struct wl_resource *resource; + + resource = wl_resource_create(client, &wl_fixes_interface, version, id); + wl_resource_set_implementation(resource, &fixes_implementation, NULL, NULL); +} + struct display_destroy_listener { struct wl_listener listener; int done; @@ -1695,6 +1752,172 @@ TEST(global_remove) display_destroy(d); } +static void +registry_ack_global_remove_handle_global_remove(void *data, + struct wl_registry *registry, + uint32_t id) +{ + struct client *client = data; + + wl_fixes_ack_global_remove(client->wl_fixes, registry, id); +} + +static struct wl_registry_listener registry_ack_global_remove_listener = { + .global = registry_handle_global_noop, + .global_remove = registry_ack_global_remove_handle_global_remove, +}; + +static void +ack_global_remove_client(void *data) +{ + struct client *c = client_connect(); + struct wl_registry *registry; + struct wl_registry_listener *registry_listener = data; + int ret; + + registry = wl_display_get_registry(c->wl_display); + wl_registry_add_listener(registry, + registry_listener, + c); + + ret = wl_display_roundtrip(c->wl_display); + assert(ret >= 0); + + /* yield the control back to the compositor so it can remove + * the global */ + assert(stop_display(c, 1) >= 0); + + /* check if there are any global_remove events */ + ret = wl_display_roundtrip(c->wl_display); + assert(ret >= 0); + + /* yield the control back to the compositor so it can check + * whether the global is withdrawn */ + assert(stop_display(c, 1) >= 0); + + wl_registry_destroy(registry); + + client_disconnect(c); +} + +static void +mark_global_withdrawn(struct wl_global *global) +{ + bool *withdrawn = wl_global_get_user_data(global); + + *withdrawn = true; +} + +TEST(ack_global_remove) +{ + struct display *d; + struct wl_global *seat; + struct wl_global *fixes; + bool withdrawn = false; + + d = display_create(); + + fixes = wl_global_create(d->wl_display, &wl_fixes_interface, + 2, NULL, bind_fixes); + + seat = wl_global_create(d->wl_display, &wl_seat_interface, + 1, &withdrawn, NULL); + + wl_global_set_withdrawn_listener(seat, mark_global_withdrawn); + + client_create(d, ack_global_remove_client, + ®istry_ack_global_remove_listener); + + display_run(d); + assert(!withdrawn); + + wl_global_remove(seat); + + /* the global will be marked as withdrawn after the client + * acknowledges the wl_registry.global_remove event */ + display_resume(d); + assert(withdrawn); + + /* the global will be still marked as withdrawn even after + * the client disconnects */ + display_resume(d); + assert(withdrawn); + + wl_global_destroy(seat); + wl_global_destroy(fixes); + + display_destroy(d); +} + +static struct wl_registry_listener registry_no_ack_global_remove_listener = { + .global = registry_handle_global_noop, + .global_remove = registry_handle_global_remove_noop, +}; + +TEST(no_ack_global_remove) +{ + struct display *d; + struct wl_global *seat; + struct wl_global *fixes; + bool withdrawn = false; + + d = display_create(); + + fixes = wl_global_create(d->wl_display, &wl_fixes_interface, + 2, NULL, bind_fixes); + + seat = wl_global_create(d->wl_display, &wl_seat_interface, + 1, &withdrawn, NULL); + + wl_global_set_withdrawn_listener(seat, mark_global_withdrawn); + + client_create(d, ack_global_remove_client, + ®istry_no_ack_global_remove_listener); + + display_run(d); + assert(!withdrawn); + + wl_global_remove(seat); + + /* if the client does not ack the wl_registry.global_remove event, + * the global will not be marked as withdrawn */ + display_resume(d); + assert(!withdrawn); + + /* when the client disconnects, the wl_registry.global_remove will + * be implicitly acknowledged and the global can be destroyed then */ + display_resume(d); + assert(withdrawn); + + wl_global_destroy(seat); + wl_global_destroy(fixes); + + display_destroy(d); +} + +TEST(global_remove_without_clients) +{ + struct display *d; + struct wl_global *global; + bool withdrawn = false; + + d = display_create(); + + global = wl_global_create(d->wl_display, &wl_seat_interface, + 1, &withdrawn, NULL); + + wl_global_set_withdrawn_listener(global, mark_global_withdrawn); + + /* if there are no clients at all, the global can be destroyed + * immediately */ + wl_global_remove(global); + assert(withdrawn); + + wl_global_destroy(global); + + display_destroy(d); +} + static void dispatch_single_read_events(struct wl_display *d) { diff --git a/tests/test-compositor.c b/tests/test-compositor.c index f412fe6e..c45d3f17 100644 --- a/tests/test-compositor.c +++ b/tests/test-compositor.c @@ -491,14 +491,15 @@ registry_handle_globals(void *data, struct wl_registry *registry, { struct client *c = data; - if (strcmp(intf, "test") != 0) - return; + if (strcmp(intf, "test") == 0) { + c->tc = wl_registry_bind(registry, id, &test_compositor_interface, ver); + assert(c->tc && "Failed binding to registry"); - c->tc = wl_registry_bind(registry, id, &test_compositor_interface, ver); - assert(c->tc && "Failed binding to registry"); - - wl_proxy_add_listener((struct wl_proxy *) c->tc, - (void *) &tc_listener, c); + wl_proxy_add_listener((struct wl_proxy *) c->tc, + (void *) &tc_listener, c); + } else if (strcmp(intf, wl_fixes_interface.name) == 0) { + c->wl_fixes = wl_registry_bind(registry, id, &wl_fixes_interface, 2); + } } static const struct wl_registry_listener registry_listener = @@ -524,6 +525,9 @@ struct client *client_connect(void) wl_display_roundtrip(c->wl_display); assert(c->tc); + if (c->wl_fixes) { + wl_fixes_destroy_registry(c->wl_fixes, reg); + } wl_registry_destroy(reg); return c; @@ -556,6 +560,10 @@ client_disconnect(struct client *c) /* check for errors */ check_error(c->wl_display); + if (c->wl_fixes) { + wl_fixes_destroy(c->wl_fixes); + } + wl_proxy_destroy((struct wl_proxy *) c->tc); wl_display_disconnect(c->wl_display); free(c); diff --git a/tests/test-compositor.h b/tests/test-compositor.h index 662a81c3..4d94f3fb 100644 --- a/tests/test-compositor.h +++ b/tests/test-compositor.h @@ -66,6 +66,7 @@ struct display { * filled. */ struct client { struct wl_display *wl_display; + struct wl_fixes *wl_fixes; struct test_compositor *tc; atomic_bool display_stopped;