Re: [PATCH v2 wayland 07/11] client: Replace the singleton zombie with bespoke zombies

2017-04-17 Thread Derek Foreman 

On 13/04/17 11:51 AM, Derek Foreman wrote:

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.

Instead of having a singleton zombie object, we add a new wl_map flag
(valid only on the client side where zombies roam - the existing
"legacy" flag was only ever used on the server side) to indicate that a
map entry is now a zombie.

We still have to refcount and potentially free the proxy immediately or
we're left with situations where it can be leaked forever.  If we clean
up on delete_id, for example, a forced disconnect will result in a leaked
proxy (breaking much of the test suite).

So, since we must free the zombied proxy immediately, we store just the
interface for it in its previous map location, so signature information
can be retained for zombie-destined events.

Signed-off-by: Derek Foreman 
---
 src/connection.c  |  20 -
 src/wayland-client.c  | 114 --
 src/wayland-private.h |   9 ++--
 src/wayland-util.c|   2 -
 4 files changed, 126 insertions(+), 19 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index f2ebe06..84f5d79 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -809,6 +809,24 @@ wl_connection_demarshal(struct wl_connection *connection,
return NULL;
 }

+bool
+wl_object_is_zombie(struct wl_map *map, uint32_t id)
+{
+   uint32_t flags;
+
+   if (map->side == WL_MAP_SERVER_SIDE)
+   return false;
+
+   if (id >= WL_SERVER_ID_START)
+   return false;
+
+   flags = wl_map_lookup_flags(map, id);
+   if (flags & WL_MAP_ENTRY_ZOMBIE)
+   return true;
+
+   return false;
+}
+
 int
 wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
 {
@@ -830,7 +848,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, 
struct wl_map *objects)
closure->args[i].o = NULL;

object = wl_map_lookup(objects, id);
-   if (object == WL_ZOMBIE_OBJECT) {
+   if (wl_object_is_zombie(objects, id)) {
/* references object we've already
 * destroyed client side */
object = NULL;
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 7243d3d..50fdfad 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;
@@ -64,6 +69,7 @@ struct wl_proxy {
void *user_data;
wl_dispatcher_func_t dispatcher;
uint32_t version;
+   struct wl_zombie *zombie;
 };

 struct wl_global {
@@ -324,6 +330,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, );
+   if (arg.type == 'h')
+   fds++;
+   }
+
+   return fds;
+}
+
+static bool
+prepare_zombie(struct wl_proxy *proxy)
+{
+   const struct wl_message *message;
+   int i, count;
+   struct wl_zombie *zombie = NULL;
+
+   for (i = 0; i < proxy->object.interface->event_count; i++) {
+   message = >object.interface->events[i];
+   count = message_count_fds(message->signature);
+   if (count && !zombie) {
+   zombie = zalloc(sizeof(struct wl_zombie));
+   if (!zombie)
+   return false;
+   zombie->event_count = 
proxy->object.interface->event_count;
+   zombie->fd_count = zalloc(zombie->event_count * 
sizeof(int));
+   if (!zombie->fd_count) {
+   free(zombie);
+   return false;
+   }
+   }
+   if (count)
+   zombie->fd_count[i] = count;
+   }
+   proxy->zombie = zombie;
+   return true;
+}
+
+static enum wl_iterator_result
+inter_zombie(void *element, void *data, uint32_t flags)
+{
+   struct wl_zombie *zombie;
+   if (!(flags & WL_MAP_ENTRY_ZOMBIE))


missing whitespace between variables and code fixed locally, will be in v3.


+   return WL_ITERATOR_CONTINUE;
+
+   if (element) {
+   zombie =

[PATCH v2 wayland 07/11] client: Replace the singleton zombie with bespoke zombies

2017-04-13 Thread Derek Foreman 
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.

Instead of having a singleton zombie object, we add a new wl_map flag
(valid only on the client side where zombies roam - the existing
"legacy" flag was only ever used on the server side) to indicate that a
map entry is now a zombie.

We still have to refcount and potentially free the proxy immediately or
we're left with situations where it can be leaked forever.  If we clean
up on delete_id, for example, a forced disconnect will result in a leaked
proxy (breaking much of the test suite).

So, since we must free the zombied proxy immediately, we store just the
interface for it in its previous map location, so signature information
can be retained for zombie-destined events.

Signed-off-by: Derek Foreman 
---
 src/connection.c  |  20 -
 src/wayland-client.c  | 114 --
 src/wayland-private.h |   9 ++--
 src/wayland-util.c|   2 -
 4 files changed, 126 insertions(+), 19 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index f2ebe06..84f5d79 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -809,6 +809,24 @@ wl_connection_demarshal(struct wl_connection *connection,
return NULL;
 }
 
+bool
+wl_object_is_zombie(struct wl_map *map, uint32_t id)
+{
+   uint32_t flags;
+
+   if (map->side == WL_MAP_SERVER_SIDE)
+   return false;
+
+   if (id >= WL_SERVER_ID_START)
+   return false;
+
+   flags = wl_map_lookup_flags(map, id);
+   if (flags & WL_MAP_ENTRY_ZOMBIE)
+   return true;
+
+   return false;
+}
+
 int
 wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
 {
@@ -830,7 +848,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, 
struct wl_map *objects)
closure->args[i].o = NULL;
 
object = wl_map_lookup(objects, id);
-   if (object == WL_ZOMBIE_OBJECT) {
+   if (wl_object_is_zombie(objects, id)) {
/* references object we've already
 * destroyed client side */
object = NULL;
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 7243d3d..50fdfad 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;
@@ -64,6 +69,7 @@ struct wl_proxy {
void *user_data;
wl_dispatcher_func_t dispatcher;
uint32_t version;
+   struct wl_zombie *zombie;
 };
 
 struct wl_global {
@@ -324,6 +330,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, );
+   if (arg.type == 'h')
+   fds++;
+   }
+
+   return fds;
+}
+
+static bool
+prepare_zombie(struct wl_proxy *proxy)
+{
+   const struct wl_message *message;
+   int i, count;
+   struct wl_zombie *zombie = NULL;
+
+   for (i = 0; i < proxy->object.interface->event_count; i++) {
+   message = >object.interface->events[i];
+   count = message_count_fds(message->signature);
+   if (count && !zombie) {
+   zombie = zalloc(sizeof(struct wl_zombie));
+   if (!zombie)
+   return false;
+   zombie->event_count = 
proxy->object.interface->event_count;
+   zombie->fd_count = zalloc(zombie->event_count * 
sizeof(int));
+   if (!zombie->fd_count) {
+   free(zombie);
+   return false;
+   }
+   }
+   if (count)
+   zombie->fd_count[i] = count;
+   }
+   proxy->zombie = zombie;
+   return true;
+}
+
+static enum wl_iterator_result
+inter_zombie(void *element, void *data, uint32_t flags)
+{
+   struct wl_zombie *zombie;
+   if (!(flags & WL_MAP_ENTRY_ZOMBIE))
+   return WL_ITERATOR_CONTINUE;
+
+   if (element) {
+   zombie = element;
+   free(zombie->fd_count);
+   free(zombie);
+   }
+
+   return