diff --git a/protocol/tests.xml b/protocol/tests.xml index 22d80a10..8f1b32ec 100644 --- a/protocol/tests.xml +++ b/protocol/tests.xml @@ -49,4 +49,35 @@ + + + + A trivial interface. + + + + + + + + Simple interface for generating server side objects to use in + zombie tests. + + + + + + + Create a new server side object. + + + + + + + + + + + diff --git a/src/connection.c b/src/connection.c index ad16eeef..310af52e 100644 --- a/src/connection.c +++ b/src/connection.c @@ -200,12 +200,6 @@ close_fds(struct wl_ring_buffer *buffer, int max) buffer->tail += size; } -void -wl_connection_close_fds_in(struct wl_connection *connection, int max) -{ - close_fds(&connection->fds_in, max); -} - int wl_connection_destroy(struct wl_connection *connection) { @@ -809,7 +803,8 @@ wl_connection_demarshal(struct wl_connection *connection, goto err; } - if (wl_map_reserve_new(objects, id) < 0) { + if (wl_map_reserve_new(objects, id) < 0 && + !wl_object_is_zombie(objects, id)) { wl_log("not a valid new object id (%u), " "message %s(%s)\n", id, message->name, message->signature); @@ -879,10 +874,6 @@ wl_object_is_zombie(struct wl_map *map, uint32_t id) 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); } @@ -909,8 +900,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 */ + /* references object we've already 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 63ce0d01..e0fb178e 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -55,11 +55,6 @@ 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; @@ -352,62 +347,11 @@ 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); + wl_proxy_unref(element); return WL_ITERATOR_CONTINUE; } @@ -471,7 +415,7 @@ static struct wl_proxy * wl_proxy_create_for_id(struct wl_proxy *factory, uint32_t id, const struct wl_interface *interface) { - struct wl_proxy *proxy; + struct wl_proxy *proxy, *zombie; struct wl_display *display = factory->display; proxy = zalloc(sizeof *proxy); @@ -484,6 +428,9 @@ wl_proxy_create_for_id(struct wl_proxy *factory, proxy->queue = factory->queue; proxy->refcount = 1; proxy->version = factory->version; + zombie = wl_map_lookup(&display->objects, id); + if (zombie && wl_object_is_zombie(&display->objects, id)) + wl_proxy_unref(zombie); wl_map_insert_at(&display->objects, 0, id, proxy); @@ -493,25 +440,21 @@ wl_proxy_create_for_id(struct wl_proxy *factory, static void proxy_destroy(struct wl_proxy *proxy) { + proxy->flags |= WL_PROXY_FLAG_DESTROYED; + 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_proxy_unref(proxy); + } else { + /* The map now contains the zombie entry. A delete_id + * event will clean up client generated ids eventually, + * server generated ids will linger until they're + * overwritten. */ 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); + proxy); } - - proxy->flags |= WL_PROXY_FLAG_DESTROYED; - - wl_proxy_unref(proxy); } static void @@ -1034,9 +977,7 @@ 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)) { - /* For zombie objects, the 'proxy' is actually the zombie - * event-information structure, which we can free. */ - free(proxy); + wl_proxy_unref(proxy); wl_map_remove(&display->objects, id); } else if (proxy) { proxy->flags |= WL_PROXY_FLAG_ID_DELETED; @@ -1411,6 +1352,7 @@ create_proxies(struct wl_proxy *sender, struct wl_closure *closure) if (proxy == NULL) return -1; closure->args[i].o = (struct wl_object *)proxy; + break; default: break; @@ -1447,6 +1389,30 @@ increase_closure_args_refcount(struct wl_closure *closure) closure->proxy->refcount++; } +static void +inter_zombie_closure(struct wl_closure *closure) +{ + const char *signature; + struct argument_details arg; + int i, count; + struct wl_proxy *proxy; + + signature = closure->message->signature; + count = arg_count_for_signature(signature); + for (i = 0; i < count; i++) { + signature = get_next_argument(signature, &arg); + switch (arg.type) { + case 'n': + proxy = (struct wl_proxy *) closure->args[i].o; + if (proxy) + proxy_destroy(proxy); + break; + default: + break; + } + } +} + static int queue_event(struct wl_display *display, int len) { @@ -1458,7 +1424,7 @@ queue_event(struct wl_display *display, int len) struct wl_event_queue *queue; struct timespec tp; unsigned int time; - int num_zombie_fds; + bool zombie = false; wl_connection_copy(display->connection, p, sizeof p); id = p[0]; @@ -1467,29 +1433,18 @@ 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). */ + /* If our proxy is gone, just eat the event. */ 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) { 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 @%d.[event %d]" + "(%d byte)\n", time / 1000, time % 1000, - zombie ? "zombie" : "unknown", - id, opcode, - num_zombie_fds, size); + id, opcode, size); } - if (num_zombie_fds > 0) - wl_connection_close_fds_in(display->connection, - num_zombie_fds); - wl_connection_consume(display->connection, size); return size; } @@ -1516,6 +1471,13 @@ queue_event(struct wl_display *display, int len) return -1; } + zombie = wl_object_is_zombie(&display->objects, id); + if (zombie) { + inter_zombie_closure(closure); + wl_closure_destroy(closure); + return size; + } + closure->proxy = proxy; increase_closure_args_refcount(closure); diff --git a/src/wayland-private.h b/src/wayland-private.h index 493e1bee..c3b28887 100644 --- a/src/wayland-private.h +++ b/src/wayland-private.h @@ -232,7 +232,4 @@ zalloc(size_t s) return calloc(1, s); } -void -wl_connection_close_fds_in(struct wl_connection *connection, int max); - #endif diff --git a/tests/display-test.c b/tests/display-test.c index 3db7c95a..2a6864ca 100644 --- a/tests/display-test.c +++ b/tests/display-test.c @@ -1629,3 +1629,134 @@ TEST(global_remove) display_destroy(d); } + +struct new_zombie_data { + struct necromancer *necromancer; + struct necromancer *necromancer_two; + struct noop *temp_noop; +}; + +static void +server_object(void *data, struct necromancer *necro, struct noop *noop) +{ + struct new_zombie_data *nzd = data; + + /* We should only see this once, since one of the two noop + * objects we create should be created as a zombie */ + assert(!nzd->temp_noop); + nzd->temp_noop = noop; +} + +struct necromancer_listener necromancer_listener = { + server_object, +}; + +static void +new_zombie_handle_globals(void *data, struct wl_registry *registry, + uint32_t id, const char *intf, uint32_t ver) +{ + struct new_zombie_data *rg = data; + + if (!strcmp(intf, "necromancer")) { + rg->necromancer = wl_registry_bind(registry, id, &necromancer_interface, 1); + rg->necromancer_two = wl_registry_bind(registry, id, &necromancer_interface, 1); + necromancer_add_listener(rg->necromancer, &necromancer_listener, NULL); + necromancer_add_listener(rg->necromancer_two, &necromancer_listener, rg); + } +} + +static const struct wl_registry_listener new_zombie_registry_listener = { + new_zombie_handle_globals, + NULL +}; + +static void noop_destructor(struct wl_client *client, struct wl_resource *res) +{ + wl_resource_destroy(res); +} + +static const struct noop_interface noop_server_interface = { + noop_destructor +}; + +static void +send_new_object(struct wl_client *client, struct wl_resource *res) +{ + struct wl_resource *new_res; + + new_res = wl_resource_create(client, &noop_interface, 1, 0); + wl_resource_set_implementation(new_res, &noop_server_interface, NULL, NULL); + necromancer_send_server_object(res, new_res); +} + +static void necromancer_destructor(struct wl_client *client, struct wl_resource *res) +{ + wl_resource_destroy(res); +} + +static const struct necromancer_interface necro_interface = { + necromancer_destructor, + send_new_object +}; + +static void +bind_necromancer(struct wl_client *client, void *data, + uint32_t vers, uint32_t id) +{ + struct wl_resource *res; + + res = wl_resource_create(client, &necromancer_interface, vers, id); + wl_resource_set_implementation(res, &necro_interface, NULL, NULL); +} + +static void +spawn_new_zombie(void *data) +{ + struct new_zombie_data rg = {}; + struct client *c = client_connect(); + struct wl_registry *reg; + + reg = wl_display_get_registry(c->wl_display); + wl_registry_add_listener(reg, &new_zombie_registry_listener, &rg); + + /* Get the globals */ + wl_display_roundtrip(c->wl_display); + + wl_registry_destroy(reg); + + necromancer_create_server_object(rg.necromancer); + necromancer_create_server_object(rg.necromancer_two); + necromancer_destroy(rg.necromancer); + /* We should now have two new_id events on the way, + * and the first one will be destined for a zombie + * when it arrives. That would violate the wl_map + * requirement that a new id can only be one higher + * than the current max id in the map if we simply + * dropped the first inbound object. + */ + + wl_display_roundtrip(c->wl_display); + assert(rg.temp_noop); + noop_destroy(rg.temp_noop); + necromancer_destroy(rg.necromancer_two); + wl_display_roundtrip(c->wl_display); + client_disconnect(c); +} + +TEST(new_zombie) +{ + struct display *d; + struct wl_global *g; + + d = display_create(); + g = wl_global_create(d->wl_display, &necromancer_interface, + 1, d, bind_necromancer); + + client_create_noarg(d, spawn_new_zombie); + + display_run(d); + + wl_global_destroy(g); + + display_destroy(d); +}