[PATCH] Keep client object reference when marking it as zombie

[Arnaud Vrac]

This will avoid a race condition on the client side when thread safety
is implemented. The destruction of a proxy might be done after the
deletion of its object id in this case: the destruction of the proxy is
done from the thread in which the object was created, and the deletion
of the object id is done in the thread the display was created.

We cannot change the object pointer in the objects map to a magic value
anymore, so instead we hack the object id on the client side.
---
 src/connection.c      |    2 +-
 src/wayland-client.c  |   49 ++++++++++++++++++++++++++++++++-----------------
 src/wayland-private.h |   10 ++++++++--
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 06cc66f..251588d 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -650,7 +650,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 			closure->args[i] = object;
 
 			*object = wl_map_lookup(objects, *p);
-			if (*object == WL_ZOMBIE_OBJECT) {
+			if (*object && wl_object_is_zombie(*object)) {
 				/* references object we've already
 				 * destroyed client side */
 				*object = NULL;
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 1a24abf..f19e40b 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -50,6 +50,7 @@ struct wl_proxy {
 	struct wl_object object;
 	struct wl_display *display;
 	void *user_data;
+	int refcount;
 };
 
 struct wl_global {
@@ -121,6 +122,28 @@ wl_display_remove_global_listener(struct wl_display *display,
 	free(listener);
 }
 
+static struct wl_proxy *
+wl_proxy_ref(struct wl_proxy *proxy)
+{
+	__sync_add_and_fetch(&proxy->refcount, 1);
+
+	return proxy;
+}
+
+static void
+wl_proxy_unref(struct wl_proxy *proxy)
+{
+	struct wl_display *display = proxy->display;
+
+	if (__sync_sub_and_fetch(&proxy->refcount, 1) > 0)
+		return;
+
+	assert(wl_object_is_zombie(&proxy->object));
+	wl_map_remove(&display->objects, proxy->object.id & ~WL_ZOMBIE_ID_MASK);
+
+	free(proxy);
+}
+
 WL_EXPORT struct wl_proxy *
 wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
 {
@@ -131,13 +154,14 @@ wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
 	if (proxy == NULL)
 		return NULL;
 
+	proxy->refcount = 1;
 	proxy->object.interface = interface;
 	proxy->object.implementation = NULL;
 	proxy->object.id = wl_map_insert_new(&display->objects,
 					     WL_MAP_CLIENT_SIDE, proxy);
 	proxy->display = display;
 
-	return proxy;
+	return wl_proxy_ref(proxy);
 }
 
 WL_EXPORT struct wl_proxy *
@@ -151,25 +175,21 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
 	if (proxy == NULL)
 		return NULL;
 
+	proxy->refcount = 1;
 	proxy->object.interface = interface;
 	proxy->object.implementation = NULL;
 	proxy->object.id = id;
 	proxy->display = display;
 	wl_map_insert_at(&display->objects, id, proxy);
 
-	return proxy;
+	return wl_proxy_ref(proxy);
 }
 
 WL_EXPORT void
 wl_proxy_destroy(struct wl_proxy *proxy)
 {
-	if (proxy->object.id < WL_SERVER_ID_START)
-		wl_map_insert_at(&proxy->display->objects,
-				 proxy->object.id, WL_ZOMBIE_OBJECT);
-	else
-		wl_map_insert_at(&proxy->display->objects,
-				 proxy->object.id, NULL);
-	free(proxy);
+	proxy->object.id |= WL_ZOMBIE_ID_MASK;
+	wl_proxy_unref(proxy);
 }
 
 WL_EXPORT int
@@ -287,10 +307,7 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
 	struct wl_proxy *proxy;
 
 	proxy = wl_map_lookup(&display->objects, id);
-	if (proxy != WL_ZOMBIE_OBJECT)
-		fprintf(stderr, "server sent delete_id for live object\n");
-	else
-		wl_map_remove(&display->objects, id);
+	wl_proxy_unref(proxy);
 }
 
 static const struct wl_display_listener display_listener = {
@@ -471,10 +488,8 @@ handle_event(struct wl_display *display,
 
 	proxy = wl_map_lookup(&display->objects, id);
 
-	if (proxy == WL_ZOMBIE_OBJECT) {
-		wl_connection_consume(display->connection, size);
-		return;
-	} else if (proxy == NULL || proxy->object.implementation == NULL) {
+	if (proxy == NULL || proxy->object.implementation == NULL ||
+	    wl_object_is_zombie(&proxy->object)) {
 		wl_connection_consume(display->connection, size);
 		return;
 	}
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 61b07a2..f4ce3e0 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -28,12 +28,18 @@
 #include <ffi.h>
 #include "wayland-util.h"
 
-#define WL_ZOMBIE_OBJECT ((void *) 2)
-
 #define WL_MAP_SERVER_SIDE 0
 #define WL_MAP_CLIENT_SIDE 1
 #define WL_SERVER_ID_START 0xff000000
 
+#define WL_ZOMBIE_ID_MASK 0xff000000
+
+static inline int
+wl_object_is_zombie(struct wl_object *object)
+{
+	return (object->id & WL_ZOMBIE_ID_MASK) != 0;
+}
+
 struct wl_map {
 	struct wl_array client_entries;
 	struct wl_array server_entries;
-- 
1.7.9.5