On 2017-12-28 01:53 PM, Daniel Stone wrote:
From: Derek Foreman <der...@osg.samsung.com>Using the singleton zombie object doesn't allow us to posthumously retain object interface information, which makes it difficult to properly inter future events destined for the recently deceased proxy. Notably, this makes it impossible for zombie proxy destined file descriptors to be properly consumed. When we create a proxy, we now create a zombie-state object to hold information about the file descriptors in events it can receive. This will allow us, in a future patch, to close those FDs. [daniels: Split Derek's patch into a few smaller ones.] Signed-off-by: Derek Foreman <der...@osg.samsung.com> Reviewed-by: Daniel Stone <dani...@collabora.com> Signed-off-by: Daniel Stone <dani...@collabora.com> --- src/wayland-client.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/src/wayland-client.c b/src/wayland-client.c index 987418a1..62d4f222 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -55,6 +55,11 @@ enum wl_proxy_flag { 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; @@ -350,6 +355,66 @@ wl_display_create_queue(struct wl_display *display) 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) @@ -434,10 +499,14 @@ 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);
I think we discussed this on irc and I said this patch was ok, but I missed this change...
prepare_zombie(proxy) should probably only be called from wl_proxy_create and create_for_id - calling it in the destroy path results in a malloc() call during deleting.
Should the malloc() fail and we can't actually allocate the zombie at during deletion we're going to have a problem.
Otherwise, I like the way you've split up the series, and I've reviewed your new patches.
Thanks, Derek
+ + /* 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, - NULL); + zombie); } else { wl_map_insert_at(&proxy->display->objects, 0, proxy->object.id, NULL); @@ -860,12 +929,16 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)proxy = wl_map_lookup(&display->objects, id); - if (wl_object_is_zombie(&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) + } else if (proxy) { proxy->flags |= WL_PROXY_FLAG_ID_DELETED; - else + } else { wl_log("error: received delete_id for unknown id (%u)\n", id); + }pthread_mutex_unlock(&display->mutex);} @@ -1087,6 +1160,7 @@ 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); wl_event_queue_release(&display->display_queue);
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
