From dcd5996175dc985f42f5307e865371cb8d4db0d4 Mon Sep 17 00:00:00 2001 From: Jonathan Leivent Date: Fri, 26 Jan 2024 18:42:14 -0500 Subject: [PATCH] util: unified and enhanced zombie handing in wl_map servers and clients now both use zombies. Zombies are interface pointers, so that they retain info for demarshalling fds AND new_id args, which must be turned into other zombies. Demarshalling messages to zombies is handled by the same wl_connection_demarshal code used for messages to live objects. Server zombies are eventually reaped after moving through FIFO zombie ring, because there is no delete_id request in the protocol. Signed-off-by: Jonathan Leivent --- src/connection.c | 66 +++++++++--- src/wayland-client.c | 199 ++++++++++++++++------------------- src/wayland-private.h | 38 ++++++- src/wayland-server.c | 76 +++++++++++--- src/wayland-util.c | 222 ++++++++++++++++++++++++++++++++++------ tests/connection-test.c | 3 + tests/map-test.c | 12 ++- 7 files changed, 436 insertions(+), 180 deletions(-) diff --git a/src/connection.c b/src/connection.c index b89166fb..ec1dd50b 100644 --- a/src/connection.c +++ b/src/connection.c @@ -689,10 +689,14 @@ wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap, } struct wl_closure * -wl_connection_demarshal(struct wl_connection *connection, +wl_connection_demarshal_common(struct wl_connection *connection, uint32_t size, struct wl_map *objects, - const struct wl_message *message) + const struct wl_message *message, + int (*handle_new_id)(void *data, + uint32_t id, + const struct wl_interface *interface), + void *data) { uint32_t *p, *next, *end, length, length_in_u32, id; int fd; @@ -813,7 +817,7 @@ wl_connection_demarshal(struct wl_connection *connection, goto err; } - if (wl_map_reserve_new(objects, id) < 0) { + if (handle_new_id(data, id, message->types[i]) < 0) { if (errno == EINVAL) { wl_log("not a valid new object id (%u), " "message %s(%s)\n", id, @@ -876,21 +880,51 @@ wl_connection_demarshal(struct wl_connection *connection, return NULL; } +static int +reserve_new_id(void *data, uint32_t id, const struct wl_interface *interface) +{ + struct wl_map *objects = data; + return wl_map_reserve_new(objects, id); +} + +struct wl_closure * +wl_connection_demarshal(struct wl_connection *connection, + uint32_t size, + struct wl_map *objects, + const struct wl_message *message) +{ + return wl_connection_demarshal_common(connection, + size, + objects, + message, + &reserve_new_id, + objects); +} + +/* demarshal messages to zombies. handle_new_id should create a new zombie. */ +void +wl_connection_demarshal_zombie(struct wl_connection *connection, + uint32_t size, + struct wl_map *objects, + const struct wl_message *message, + int (*handle_new_id)(void* data, + uint32_t id, + const struct wl_interface *interface), + void *data) +{ + struct wl_closure *closure = wl_connection_demarshal_common(connection, + size, + objects, + message, + handle_new_id, + data); + wl_closure_destroy(closure); +} + bool wl_object_is_zombie(struct wl_map *map, uint32_t id) { - uint32_t flags; - - /* Zombie objects only exist on the client side. */ - if (map->side == WL_MAP_SERVER_SIDE) - return false; - - /* Zombie objects can only have been created by the client. */ - if (id >= WL_SERVER_ID_START) - return false; - - flags = wl_map_lookup_flags(map, id); - return !!(flags & WL_MAP_ENTRY_ZOMBIE); + return wl_map_lookup_zombie(map, id) != NULL; } int @@ -916,7 +950,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects) object = wl_map_lookup(objects, id); if (wl_object_is_zombie(objects, id)) { /* references object we've already - * destroyed client side */ + * destroyed */ object = NULL; } else if (object == NULL && id != 0) { wl_log("unknown object (%u), message %s(%s)\n", diff --git a/src/wayland-client.c b/src/wayland-client.c index 94438e30..6f01fa99 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -50,16 +50,10 @@ /** \cond */ enum wl_proxy_flag { - WL_PROXY_FLAG_ID_DELETED = (1 << 0), WL_PROXY_FLAG_DESTROYED = (1 << 1), WL_PROXY_FLAG_WRAPPER = (1 << 2), }; -struct wl_zombie { - int event_count; - int *fd_count; -}; - struct wl_proxy { struct wl_object object; struct wl_display *display; @@ -412,66 +406,6 @@ wl_display_create_queue_with_name(struct wl_display *display, const char *name) return queue; } -static int -message_count_fds(const char *signature) -{ - unsigned int count, i, fds = 0; - struct argument_details arg; - - count = arg_count_for_signature(signature); - for (i = 0; i < count; i++) { - signature = get_next_argument(signature, &arg); - if (arg.type == 'h') - fds++; - } - - return fds; -} - -static struct wl_zombie * -prepare_zombie(struct wl_proxy *proxy) -{ - const struct wl_interface *interface = proxy->object.interface; - const struct wl_message *message; - int i, count; - struct wl_zombie *zombie = NULL; - - /* If we hit an event with an FD, ensure we have a zombie object and - * fill the fd_count slot for that event with the number of FDs for - * that event. Interfaces with no events containing FDs will not have - * zombie objects created. */ - for (i = 0; i < interface->event_count; i++) { - message = &interface->events[i]; - count = message_count_fds(message->signature); - - if (!count) - continue; - - if (!zombie) { - zombie = zalloc(sizeof(*zombie) + - (interface->event_count * sizeof(int))); - if (!zombie) - return NULL; - - zombie->event_count = interface->event_count; - zombie->fd_count = (int *) &zombie[1]; - } - - zombie->fd_count[i] = count; - } - - return zombie; -} - -static enum wl_iterator_result -free_zombies(void *element, void *data, uint32_t flags) -{ - if (flags & WL_MAP_ENTRY_ZOMBIE) - free(element); - - return WL_ITERATOR_CONTINUE; -} - static struct wl_proxy * proxy_create(struct wl_proxy *factory, const struct wl_interface *interface, uint32_t version) @@ -564,21 +498,9 @@ wl_proxy_create_for_id(struct wl_proxy *factory, static void proxy_destroy(struct wl_proxy *proxy) { - if (proxy->flags & WL_PROXY_FLAG_ID_DELETED) { - wl_map_remove(&proxy->display->objects, proxy->object.id); - } else if (proxy->object.id < WL_SERVER_ID_START) { - struct wl_zombie *zombie = prepare_zombie(proxy); - - /* The map now contains the zombie entry, until the delete_id - * event arrives. */ - wl_map_insert_at(&proxy->display->objects, - WL_MAP_ENTRY_ZOMBIE, - proxy->object.id, - zombie); - } else { - wl_map_insert_at(&proxy->display->objects, 0, - proxy->object.id, NULL); - } + wl_map_zombify(&proxy->display->objects, + proxy->object.id, + proxy->object.interface); proxy->flags |= WL_PROXY_FLAG_DESTROYED; @@ -1107,22 +1029,10 @@ display_handle_error(void *data, static void display_handle_delete_id(void *data, struct wl_display *display, uint32_t id) { - struct wl_proxy *proxy; - pthread_mutex_lock(&display->mutex); - proxy = wl_map_lookup(&display->objects, id); - - if (wl_object_is_zombie(&display->objects, id)) { - /* For zombie objects, the 'proxy' is actually the zombie - * event-information structure, which we can free. */ - free(proxy); - wl_map_remove(&display->objects, id); - } else if (proxy) { - proxy->flags |= WL_PROXY_FLAG_ID_DELETED; - } else { + if (wl_map_mark_deleted(&display->objects, id) != 0) wl_log("error: received delete_id for unknown id (%u)\n", id); - } pthread_mutex_unlock(&display->mutex); } @@ -1359,7 +1269,6 @@ WL_EXPORT void wl_display_disconnect(struct wl_display *display) { wl_connection_destroy(display->connection); - wl_map_for_each(&display->objects, free_zombies, NULL); wl_map_release(&display->objects); wl_event_queue_release(&display->default_queue); free(display->default_queue.name); @@ -1537,6 +1446,40 @@ increase_closure_args_refcount(struct wl_closure *closure) closure->proxy->refcount++; } +/* zombify_new_id is only called by wl_connection_demarshal_zombie + * when the event it is demarhsalling has new_id argments, meaning + * they're server IDs. Those server IDs need to have zombies created + * for them in case the server subsequently sends an event to them, + * which might contain fds or more new_ids. */ +static int +zombify_new_id(void *displayp, uint32_t id, const struct wl_interface *interface) +{ + struct wl_display *display = displayp; + + if (id >= WL_SERVER_ID_START) { + /* Make the server ID a zombie in case it gets sent an + * event. This handles the "domino effect" case where + * new_id args in events sent to zombies are + * subsequently sent events, with new_id args, and so + * on. */ + if (wl_map_insert_at(&display->objects, 0, id, (void*)interface) != 0) { + errno = EINVAL; + return -1; + } + if (wl_map_zombify(&display->objects, id, interface) != 0) { + errno = EINVAL; + return -1; + } + assert(wl_map_lookup_zombie(&display->objects, id) == interface); + + return 0; + } else { + /* Unexpected: the id is a client ID */ + errno = EINVAL; + return -1; + } +} + static int queue_event(struct wl_display *display, int len) { @@ -1548,7 +1491,7 @@ queue_event(struct wl_display *display, int len) struct wl_event_queue *queue; struct timespec tp; unsigned int time; - int num_zombie_fds; + const struct wl_interface *interface; wl_connection_copy(display->connection, p, sizeof p); id = p[0]; @@ -1557,30 +1500,66 @@ queue_event(struct wl_display *display, int len) if (len < size) return 0; - /* If our proxy is gone or a zombie, just eat the event (and any FDs, - * if applicable). */ proxy = wl_map_lookup(&display->objects, id); - if (!proxy || wl_object_is_zombie(&display->objects, id)) { - struct wl_zombie *zombie = wl_map_lookup(&display->objects, id); - num_zombie_fds = (zombie && opcode < zombie->event_count) ? - zombie->fd_count[opcode] : 0; + if (!proxy) { + interface = wl_map_lookup_zombie(&display->objects, id); if (debug_client) { clock_gettime(CLOCK_REALTIME, &tp); time = (tp.tv_sec * 1000000L) + (tp.tv_nsec / 1000); - fprintf(stderr, "[%7u.%03u] discarded [%s]#%d.[event %d]" - "(%d fd, %d byte)\n", + fprintf(stderr, "[%7u.%03u] discarded [%s]#%u.[event %d]" + "(%s, %d byte)\n", time / 1000, time % 1000, - zombie ? "zombie" : "unknown", + interface ? "zombie" : "unknown", id, opcode, - num_zombie_fds, size); + interface ? interface->name : "unknown interface", + size); } - if (num_zombie_fds > 0) - wl_connection_close_fds_in(display->connection, - num_zombie_fds); - wl_connection_consume(display->connection, size); + if (interface) { + if (opcode >= interface->event_count) { + wl_log("interface '%s' has no event %u\n", + interface->name, opcode); + return -1; + } + message = &interface->events[opcode]; + if (message == NULL) { + wl_log("interface '%s' opcode %u has no event\n", + interface->name, opcode); + return -1; + } + if (message->name == NULL) { + wl_log("interface '%s' opcode %u has noname event,\n", + interface->name, opcode); + return -1; + } + wl_connection_demarshal_zombie(display->connection, size, + &display->objects, + message, + &zombify_new_id, display); + } else { + /* what to do if there is no zombie interface? The + * original zombie code just consumed the size and + * continued on. The assumption was if there was no + * zombie, then the reason was that there's no fds to + * consume, because zombies were only created for proxies + * that have fd-bearing events. But, there may have been + * new_id args, so that could still have caused problems. + * Under the new zombie regime, which always creates + * zombies for proxies (because even an unpatched server + * sends delete_id events), the absence of a zombie is + * unexpected and means we need to kill the client. */ + wl_log("Missing zombie for ID %u, event %u\n", id, opcode); + return -1; + /* Consider the case in the old regime where the proxy had + * no fd-bearing events and no new_id-bearing events. In + * such cases, when there was no zombie, consuming the + * size and moving on was OK, but there was no way to tell + * it was OK. Now we know the difference, and will always + * log this error. + */ + } return size; } diff --git a/src/wayland-private.h b/src/wayland-private.h index 6b506584..eb67dc5e 100644 --- a/src/wayland-private.h +++ b/src/wayland-private.h @@ -59,19 +59,23 @@ wl_interface_equal(const struct wl_interface *iface1, const struct wl_interface *iface2); /* Flags for wl_map_insert_new and wl_map_insert_at. Flags can be queried with - * wl_map_lookup_flags. The current implementation has room for 1 bit worth of + * wl_map_lookup_flags. The current implementation has room for 32 bits worth of * flags. If more flags are ever added, the implementation of wl_map will have * to change to allow for new flags */ enum wl_map_entry_flags { WL_MAP_ENTRY_LEGACY = (1 << 0), /* Server side only */ - WL_MAP_ENTRY_ZOMBIE = (1 << 0) /* Client side only */ }; struct wl_map { struct wl_array client_entries; struct wl_array server_entries; uint32_t side; + /* the free_list is LIFO, the zombie list is FIFO */ uint32_t free_list; + uint32_t zombie_list_head; + uint32_t zombie_list_tail; + /* if zombie_list_count is < 0, then the zombie_list is disabled */ + int32_t zombie_list_count; }; typedef enum wl_iterator_result (*wl_iterator_func_t)(void *element, @@ -93,12 +97,18 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data); int wl_map_reserve_new(struct wl_map *map, uint32_t i); -void -wl_map_remove(struct wl_map *map, uint32_t i); +int +wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interface); + +int +wl_map_mark_deleted(struct wl_map *map, uint32_t i); void * wl_map_lookup(struct wl_map *map, uint32_t i); +const struct wl_interface * +wl_map_lookup_zombie(struct wl_map *map, uint32_t i); + uint32_t wl_map_lookup_flags(struct wl_map *map, uint32_t i); @@ -179,6 +189,26 @@ wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap, const struct wl_message *message); +struct wl_closure * +wl_connection_demarshal_common(struct wl_connection *connection, + uint32_t size, + struct wl_map *objects, + const struct wl_message *message, + int (*handle_new_id)(void *data, + uint32_t id, + const struct wl_interface *interface), + void *data); + +void +wl_connection_demarshal_zombie(struct wl_connection *connection, + uint32_t size, + struct wl_map *objects, + const struct wl_message *message, + int (*handle_new_id)(void* data, + uint32_t id, + const struct wl_interface *interface), + void *data); + struct wl_closure * wl_connection_demarshal(struct wl_connection *connection, uint32_t size, diff --git a/src/wayland-server.c b/src/wayland-server.c index 78ff4055..38427f39 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -327,6 +327,38 @@ destroy_client_with_error(struct wl_client *client, const char *reason) wl_client_destroy(client); } +/* zombify_new_id_queue_delete_id_event is only called by + * wl_connection_demarshal_zombie when the request it is demarhsalling + * has new_id argments, meaning they're client IDs. Those client IDs + * need to have zombies created for them in case the client + * subsequently sends a request to them, which might contain fds or + * more new_ids. */ +static int +zombify_new_id_queue_delete_id_event(void *clientp, uint32_t id, + const struct wl_interface *interface) +{ + struct wl_client *client = clientp; + + if (id < WL_SERVER_ID_START) { + if (wl_map_insert_at(&client->objects, 0, id, (void*)interface) != 0) { + errno = EINVAL; + return -1; + } + if (wl_map_zombify(&client->objects, id, interface) != 0) { + errno = EINVAL; + return -1; + } + if (client->display_resource) + wl_resource_queue_event(client->display_resource, + WL_DISPLAY_DELETE_ID, id); + + return 0; + } else { + /* Unexpected: the id is a server ID. */ + return -1; + } +} + static int wl_client_connection_data(int fd, uint32_t mask, void *data) { @@ -340,6 +372,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) uint32_t resource_flags; int opcode, size, since; int len; + const struct wl_interface *interface; if (mask & WL_EVENT_HANGUP) { wl_client_destroy(client); @@ -383,9 +416,29 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) resource = wl_map_lookup(&client->objects, p[0]); resource_flags = wl_map_lookup_flags(&client->objects, p[0]); if (resource == NULL) { - wl_resource_post_error(client->display_resource, - WL_DISPLAY_ERROR_INVALID_OBJECT, - "invalid object %u", p[0]); + interface = wl_map_lookup_zombie(&client->objects, p[0]); + if (interface == NULL) { + wl_resource_post_error(client->display_resource, + WL_DISPLAY_ERROR_INVALID_OBJECT, + "invalid object %u", p[0]); + break; + } + + if (opcode >= interface->method_count) { + wl_resource_post_error(client->display_resource, + WL_DISPLAY_ERROR_INVALID_METHOD, + "invalid method %d, object %s#%u", + opcode, + interface->name, + p[0]); + break; + } + + message = &interface->methods[opcode]; + wl_connection_demarshal_zombie(client->connection, size, + &client->objects, message, + &zombify_new_id_queue_delete_id_event, + client); break; } @@ -735,7 +788,8 @@ 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;; + uint32_t id = resource->object.id; + const struct wl_interface *interface = resource->object.interface; wl_signal_emit(&resource->deprecated_destroy_signal, resource); /* Don't emit the new signal for deprecated resources, as that would @@ -746,16 +800,12 @@ remove_and_destroy_resource(void *element, void *data, uint32_t flags) if (resource->destroy) resource->destroy(resource); + if (id < WL_SERVER_ID_START && client->display_resource) + wl_resource_queue_event(client->display_resource, + WL_DISPLAY_DELETE_ID, id); + /* 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); - } + wl_map_zombify(&client->objects, id, interface); if (!(flags & WL_MAP_ENTRY_LEGACY)) free(resource); diff --git a/src/wayland-util.c b/src/wayland-util.c index bb2a1838..1fd4fcfc 100644 --- a/src/wayland-util.c +++ b/src/wayland-util.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "wayland-util.h" #include "wayland-private.h" @@ -169,20 +170,48 @@ wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b) return a == b || strcmp(a->name, b->name) == 0; } -union map_entry { - uintptr_t next; +struct map_entry { + uint32_t next; + uint32_t flags : 8*sizeof(uint32_t) - 3; + uint32_t zombie : 1; + uint32_t freelisted : 1; + uint32_t deleted : 1; void *data; }; -#define map_entry_is_free(entry) ((entry).next & 0x1) -#define map_entry_get_data(entry) ((void *)((entry).next & ~(uintptr_t)0x3)) -#define map_entry_get_flags(entry) (((entry).next >> 1) & 0x1) +int32_t max_zombie_list_count = 64; + +/* We cannot use NULL (0) to denote null links in the free list or + zombie list because index 0 is allowed on the server side: id == + WL_SERVER_ID_START --> index == 0. Instead, use: */ +static const uint32_t map_null_link = ~0; + +static inline int +map_entry_is_free(struct map_entry *entry) +{ + return entry->zombie || entry->freelisted; +} + +static inline void +map_entry_clear(struct map_entry *entry) +{ + entry->next = map_null_link; + entry->flags = 0; + entry->zombie = 0; + entry->freelisted = 0; + entry->deleted = 0; + entry->data = NULL; +} void wl_map_init(struct wl_map *map, uint32_t side) { memset(map, 0, sizeof *map); map->side = side; + map->free_list = map_null_link; + map->zombie_list_head = map_null_link; + map->zombie_list_tail = map_null_link; + map->zombie_list_count = 0; } void @@ -195,7 +224,7 @@ wl_map_release(struct wl_map *map) uint32_t wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data) { - union map_entry *start, *entry; + struct map_entry *start, *entry; struct wl_array *entries; uint32_t base; uint32_t count; @@ -208,9 +237,10 @@ wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data) base = WL_SERVER_ID_START; } - if (map->free_list) { + if (map->free_list != map_null_link) { start = entries->data; - entry = &start[map->free_list >> 1]; + entry = &start[map->free_list]; + assert(entry->freelisted); map->free_list = entry->next; } else { entry = wl_array_add(entries, sizeof *entry); @@ -235,20 +265,21 @@ wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data) errno = ENOSPC; return 0; } + map_entry_clear(entry); entry->data = data; - entry->next |= (flags & 0x1) << 1; - + entry->flags = flags; return count + base; } int wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data) { - union map_entry *start; + struct map_entry *start; uint32_t count; struct wl_array *entries; if (i < WL_SERVER_ID_START) { + assert(i == 0 || map->side == WL_MAP_SERVER_SIDE); entries = &map->client_entries; } else { entries = &map->server_entries; @@ -272,8 +303,10 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data) } start = entries->data; + + map_entry_clear(&start[i]); start[i].data = data; - start[i].next |= (flags & 0x1) << 1; + start[i].flags = flags; return 0; } @@ -281,7 +314,7 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data) int wl_map_reserve_new(struct wl_map *map, uint32_t i) { - union map_entry *start; + struct map_entry *start; uint32_t count; struct wl_array *entries; @@ -318,10 +351,16 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i) return -1; start = entries->data; - start[i].data = NULL; + map_entry_clear(&start[i]); } else { start = entries->data; - if (start[i].data != NULL) { + + assert(!start[i].freelisted); + + /* In the new zombie regime, there may be zombies in + * any map, even opposite side ones. So a test here + * for start[i].data != NULL is no longer right.*/ + if (!map_entry_is_free(&start[i])) { errno = EINVAL; return -1; } @@ -330,34 +369,128 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i) return 0; } -void -wl_map_remove(struct wl_map *map, uint32_t i) +int +wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interface) { - union map_entry *start; + struct map_entry *start, *entry; struct wl_array *entries; + bool use_zombie_list; + uint32_t count; + static bool checked_env = false; + const char *max_var; + + assert(i != 0); + + if (i < WL_SERVER_ID_START) { + use_zombie_list = false; + entries = &map->client_entries; + } else { + use_zombie_list = (map->side == WL_MAP_SERVER_SIDE) && + (map->zombie_list_count >= 0); + entries = &map->server_entries; + i -= WL_SERVER_ID_START; + } + + start = entries->data; + count = entries->size / sizeof *start; + + if (i >= count) + return -1; + + if (start[i].deleted) { + /* No need to make this entry into a zombie if it has + already been in a delete_id message - put it on the + free list instead */ + start[i].next = map->free_list; + start[i].freelisted = 1; + map->free_list = i; + return 0; + } + + start[i].data = (void*)interface; + start[i].zombie = 1; + start[i].next = map_null_link; + + if (use_zombie_list) { + if (!checked_env) { + checked_env = true; + max_var = getenv("WAYLAND_MAX_ZOMBIE_LIST_COUNT"); + if (max_var && *max_var) + max_zombie_list_count = atoi(max_var); + } + + if (map->zombie_list_tail != map_null_link) + start[map->zombie_list_tail].next = i; + else + map->zombie_list_head = i; + map->zombie_list_tail = i; + map->zombie_list_count++; + + if (map->zombie_list_count > max_zombie_list_count) { + i = map->zombie_list_head; + entry = &start[i]; + map->zombie_list_head = entry->next; + if (map->zombie_list_head == map_null_link) + map->zombie_list_tail = map_null_link; + map->zombie_list_count--; + + entry->next = map->free_list; + entry->freelisted = 1; + entry->zombie = 0; + map->free_list = i; + } + } + return 0; +} + +int +wl_map_mark_deleted(struct wl_map *map, uint32_t i) +{ + struct map_entry *start; + struct wl_array *entries; + uint32_t count; + + assert(i != 0); if (i < WL_SERVER_ID_START) { if (map->side == WL_MAP_SERVER_SIDE) - return; + return 0; entries = &map->client_entries; } else { if (map->side == WL_MAP_CLIENT_SIDE) - return; + return 0; entries = &map->server_entries; i -= WL_SERVER_ID_START; } start = entries->data; - start[i].next = map->free_list; - map->free_list = (i << 1) | 1; + count = entries->size / sizeof *start; + + if (i >= count) + return -1; + + /* turn off the zombie list - because the zombie_list is not + needed if we are receiving delete_id messages, and is + incompatible with randomly moving zombies directly to the + free list */ + map->zombie_list_count = -1; + + start[i].deleted = 1; + if (start[i].zombie) { + start[i].next = map->free_list; + start[i].freelisted = 1; + start[i].zombie = 0; + map->free_list = i; + } + return 0; } void * wl_map_lookup(struct wl_map *map, uint32_t i) { - union map_entry *start; + struct map_entry *start; uint32_t count; struct wl_array *entries; @@ -371,8 +504,31 @@ wl_map_lookup(struct wl_map *map, uint32_t i) start = entries->data; count = entries->size / sizeof *start; - if (i < count && !map_entry_is_free(start[i])) - return map_entry_get_data(start[i]); + if (i < count && !map_entry_is_free(&start[i])) + return start[i].data; + + return NULL; +} + +const struct wl_interface * +wl_map_lookup_zombie(struct wl_map *map, uint32_t i) +{ + struct map_entry *start; + uint32_t count; + struct wl_array *entries; + + if (i < WL_SERVER_ID_START) { + entries = &map->client_entries; + } else { + entries = &map->server_entries; + i -= WL_SERVER_ID_START; + } + + start = entries->data; + count = entries->size / sizeof *start; + + if (i < count && start[i].zombie) + return start[i].data; return NULL; } @@ -380,7 +536,7 @@ wl_map_lookup(struct wl_map *map, uint32_t i) uint32_t wl_map_lookup_flags(struct wl_map *map, uint32_t i) { - union map_entry *start; + struct map_entry *start; uint32_t count; struct wl_array *entries; @@ -394,8 +550,8 @@ wl_map_lookup_flags(struct wl_map *map, uint32_t i) start = entries->data; count = entries->size / sizeof *start; - if (i < count && !map_entry_is_free(start[i])) - return map_entry_get_flags(start[i]); + if (i < count && !map_entry_is_free(&start[i])) + return start[i].flags; return 0; } @@ -404,16 +560,16 @@ static enum wl_iterator_result for_each_helper(struct wl_array *entries, wl_iterator_func_t func, void *data) { enum wl_iterator_result ret = WL_ITERATOR_CONTINUE; - union map_entry entry, *start; + struct map_entry entry, *start; size_t count; - start = (union map_entry *) entries->data; - count = entries->size / sizeof(union map_entry); + start = (struct map_entry *) entries->data; + count = entries->size / sizeof(struct map_entry); for (size_t idx = 0; idx < count; idx++) { entry = start[idx]; - if (entry.data && !map_entry_is_free(entry)) { - ret = func(map_entry_get_data(entry), data, map_entry_get_flags(entry)); + if (entry.data && !map_entry_is_free(&entry)) { + ret = func(entry.data, data, entry.flags); if (ret != WL_ITERATOR_CONTINUE) break; } diff --git a/tests/connection-test.c b/tests/connection-test.c index 9762e0da..f97936d8 100644 --- a/tests/connection-test.c +++ b/tests/connection-test.c @@ -393,6 +393,7 @@ demarshal(struct marshal_data *data, const char *format, assert(closure); wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data); wl_closure_destroy(closure); + wl_map_release(&objects); } TEST(connection_demarshal) @@ -476,6 +477,7 @@ marshal_demarshal(struct marshal_data *data, assert(closure); wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data); wl_closure_destroy(closure); + wl_map_release(&objects); } TEST(connection_marshal_demarshal) @@ -540,6 +542,7 @@ expected_fail_demarshal(struct marshal_data *data, const char *format, assert(closure == NULL); assert(errno == expected_error); + wl_map_release(&objects); } TEST(connection_demarshal_null_strings) diff --git a/tests/map-test.c b/tests/map-test.c index 03568ea7..77088c85 100644 --- a/tests/map-test.c +++ b/tests/map-test.c @@ -89,7 +89,8 @@ TEST(map_remove) assert(wl_map_lookup(&map, j) == &b); assert(wl_map_lookup(&map, k) == &c); - wl_map_remove(&map, j); + wl_map_mark_deleted(&map, j); + wl_map_zombify(&map, j, NULL); assert(wl_map_lookup(&map, j) == NULL); /* Verify that we insert d at the hole left by removing b */ @@ -107,15 +108,18 @@ TEST(map_flags) wl_map_init(&map, WL_MAP_SERVER_SIDE); i = wl_map_insert_new(&map, 0, &a); - j = wl_map_insert_new(&map, 1, &b); + uint32_t flag_value = 0xabcdef10; + /* 3 bits of flags are used internally, so we lose the high 3 here: */ + uint32_t high_truncated_flag_value = (flag_value << 3) >> 3; + j = wl_map_insert_new(&map, high_truncated_flag_value, &b); assert(i == WL_SERVER_ID_START); assert(j == WL_SERVER_ID_START + 1); assert(wl_map_lookup(&map, i) == &a); assert(wl_map_lookup(&map, j) == &b); - assert(wl_map_lookup_flags(&map, i) == 0); - assert(wl_map_lookup_flags(&map, j) == 1); + assert(wl_map_lookup_flags(&map, i) == 0); + assert(wl_map_lookup_flags(&map, j) == high_truncated_flag_value); wl_map_release(&map); }