diff --git a/src/wayland-server.c b/src/wayland-server.c index 1534114b..9fd12273 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -717,10 +717,23 @@ resource_is_deprecated(struct wl_resource *resource) return false; } +/** Removes the wl_resource from the client's object map and deletes it. + * + * Triggers the destroy signal and destructor for the resource before + * removing it from the client's object map and releasing the resource's + * memory. + * + * This order is important to ensure listeners and destruction code can + * find the resource before it has been destroyed whilst ensuring the + * resource is not accessible via the object map after memory has been + * freed. + */ static enum wl_iterator_result -destroy_resource(void *element, void *data, uint32_t flags) +remove_and_destroy_resource(void *element, void *data, uint32_t flags) { struct wl_resource *resource = element; + struct wl_client *client = resource->client; + uint32_t id = resource->object.id;; wl_signal_emit(&resource->deprecated_destroy_signal, resource); /* Don't emit the new signal for deprecated resources, as that would @@ -731,6 +744,17 @@ destroy_resource(void *element, void *data, uint32_t flags) if (resource->destroy) resource->destroy(resource); + /* The resource should be cleared from the map before memory is freed. */ + if (id < WL_SERVER_ID_START) { + if (client->display_resource) { + wl_resource_queue_event(client->display_resource, + WL_DISPLAY_DELETE_ID, id); + } + wl_map_insert_at(&client->objects, 0, id, NULL); + } else { + wl_map_remove(&client->objects, id); + } + if (!(flags & WL_MAP_ENTRY_LEGACY)) free(resource); @@ -741,22 +765,9 @@ WL_EXPORT void wl_resource_destroy(struct wl_resource *resource) { struct wl_client *client = resource->client; - uint32_t id; - uint32_t flags; + uint32_t flags = wl_map_lookup_flags(&client->objects, resource->object.id); - id = resource->object.id; - flags = wl_map_lookup_flags(&client->objects, id); - destroy_resource(resource, NULL, flags); - - if (id < WL_SERVER_ID_START) { - if (client->display_resource) { - wl_resource_queue_event(client->display_resource, - WL_DISPLAY_DELETE_ID, id); - } - wl_map_insert_at(&client->objects, 0, id, NULL); - } else { - wl_map_remove(&client->objects, id); - } + remove_and_destroy_resource(resource, NULL, flags); } WL_EXPORT uint32_t @@ -920,12 +931,10 @@ wl_client_get_destroy_late_listener(struct wl_client *client, WL_EXPORT void wl_client_destroy(struct wl_client *client) { - uint32_t serial = 0; - wl_priv_signal_final_emit(&client->destroy_signal, client); wl_client_flush(client); - wl_map_for_each(&client->objects, destroy_resource, &serial); + wl_map_for_each(&client->objects, remove_and_destroy_resource, NULL); wl_map_release(&client->objects); wl_event_source_remove(client->source); close(wl_connection_destroy(client->connection)); diff --git a/tests/resources-test.c b/tests/resources-test.c index fa6ba2b2..92707297 100644 --- a/tests/resources-test.c +++ b/tests/resources-test.c @@ -206,3 +206,60 @@ TEST(free_without_remove) assert(a.link.next == a.link.prev && a.link.next == NULL); assert(b.link.next == b.link.prev && b.link.next == NULL); } + +static enum wl_iterator_result +client_resource_check(struct wl_resource* resource, void* data) +{ + /* Ensure there is no iteration over already freed resources. */ + assert(!wl_resource_get_user_data(resource)); + return WL_ITERATOR_CONTINUE; +} + +static void +resource_destroy_notify(struct wl_listener *l, void *data) +{ + struct wl_resource* resource = data; + struct wl_client* client = resource->client; + + wl_client_for_each_resource(client, client_resource_check, NULL); + + /* Set user data to flag the resource has been deleted. The resource should + * not be accessible from this point forward. */ + wl_resource_set_user_data(resource, client); +} + +TEST(resource_destroy_iteration) +{ + struct wl_display *display; + struct wl_client *client; + struct wl_resource *resource1; + struct wl_resource *resource2; + int s[2]; + + struct wl_listener destroy_listener1 = { + .notify = &resource_destroy_notify + }; + struct wl_listener destroy_listener2 = { + .notify = &resource_destroy_notify + }; + + assert(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, s) == 0); + display = wl_display_create(); + assert(display); + client = wl_client_create(display, s[0]); + assert(client); + resource1 = wl_resource_create(client, &wl_callback_interface, 1, 0); + resource2 = wl_resource_create(client, &wl_callback_interface, 1, 0); + assert(resource1); + assert(resource2); + + wl_resource_add_destroy_listener(resource1, &destroy_listener1); + wl_resource_add_destroy_listener(resource2, &destroy_listener2); + + wl_client_destroy(client); + + close(s[0]); + close(s[1]); + + wl_display_destroy(display); +}