[RFC PATCH wayland v2 3/4] server: implement inert resources

Marek Chalupa mchqwerty at gmail.com
Fri Apr 10 06:45:40 PDT 2015


When client sends request with new_id argument to the object
that has been destroyed on server-side just before that (client
may not know about it, because it has not got the information
via wire yet), we have to create the resource. If we wouldn't do it then
client will get invalid error id once it tries to use the new object.
But if we create it, then we have to take care that all the requests
but destructor are ignored, because we do not have the server-side
object anymore. Client will destroy the resource right away, because
it gets the information about server-side object destruction.

This patch solves this ugly race by adding wl_resource_set_inert()
function that marks the resource as inert. When resource is inert
it ignores all requests and events but destructor. The trick is in
adding flag into the request/even siganture that says: "hey! I'm
destructor". Server then can skip non-destructor actions on inert
resource.

When an object asks for creating new object from inert object,
wayland will silently create zombie object without invoking
appropriate handler, so once programmer marks resource as inert,
wayland will take care of the rest.

Programmer then can mark newly created resource as inert when
this race come up instead of defining new implementation of
resource just for this rare case.

v2: s/intact/inert
    requests with new_id on inert objects create zombie objects

Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
---
 src/connection.c      |  61 +++++++++++++++++++++-
 src/scanner.c         |   5 ++
 src/wayland-client.c  |   7 +++
 src/wayland-private.h |  16 +++++-
 src/wayland-server.c  | 142 +++++++++++++++++++++++++++++++++++++++++---------
 src/wayland-server.h  |   6 +++
 6 files changed, 207 insertions(+), 30 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 2545194..87ada61 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -415,6 +415,9 @@ get_next_argument(const char *signature, struct argument_details *details)
 	details->nullable = 0;
 	for(; *signature; ++signature) {
 		switch(*signature) {
+		case 'D':
+			/* skip destructor flag */
+			break;
 		case 'i':
 		case 'u':
 		case 'f':
@@ -457,8 +460,14 @@ int
 wl_message_get_since(const struct wl_message *message)
 {
 	int since;
+	const char *sig = message->signature;
+
+	/* destructor flag is always the first letter in
+	 * signature */
+	if (*sig == 'D')
+		++sig;
 
-	since = atoi(message->signature);
+	since = atoi(sig);
 
 	if (since == 0)
 		since = 1;
@@ -608,6 +617,20 @@ wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap,
 	return wl_closure_marshal(sender, opcode, args, message);
 }
 
+static struct wl_object *
+create_inert_object(const struct wl_interface *intf, uint32_t id)
+{
+	struct wl_object *obj = calloc(1, sizeof *obj);
+	if (!obj)
+		return NULL;
+
+	obj->interface = intf;
+	obj->id = id;
+	obj->flags = (WL_OBJECT_FLAG_INERT | WL_OBJECT_FLAG_INERT_INHERENTLY);
+
+	return obj;
+}
+
 struct wl_closure *
 wl_connection_demarshal(struct wl_connection *connection,
 			uint32_t size,
@@ -622,6 +645,8 @@ wl_connection_demarshal(struct wl_connection *connection,
 	struct argument_details arg;
 	struct wl_closure *closure;
 	struct wl_array *array, *array_extra;
+	struct wl_object *sender, *iobj;
+	const struct wl_interface *interface;
 
 	count = arg_count_for_signature(message->signature);
 	if (count > WL_CLOSURE_MAX_ARGS) {
@@ -732,6 +757,37 @@ wl_connection_demarshal(struct wl_connection *connection,
 				goto err;
 			}
 
+			sender = wl_map_lookup(objects, closure->sender_id);
+			if(!sender) {
+				wl_log("got message from unknown object (%d)\n",
+				       closure->sender_id);
+				goto err;
+			}
+
+			if (sender->flags & WL_OBJECT_FLAG_INERT) {
+				interface = message->types[i];
+				if (!interface) {
+					wl_log("new_id argument in message %s(%s)"
+					       " has NULL interface\n",
+					       message->name, message->signature);
+					errno = EINVAL;
+					goto err;
+				}
+
+				iobj = create_inert_object(interface, id);
+				if (!iobj) {
+					errno = ENOMEM;
+					goto err;
+				}
+
+				if(wl_map_insert_at(objects, 0, id, iobj) < 0) {
+					/* we reserved new id just few lines above,
+					 * so this insert_at must never fail. If
+					 * it does, it is a bug in wayland */
+					assert(0 && "BUG in wl_map implementation");
+				}
+			}
+
 			break;
 		case 'a':
 			length = *p++;
@@ -1186,9 +1242,10 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
 	clock_gettime(CLOCK_REALTIME, &tp);
 	time = (tp.tv_sec * 1000000L) + (tp.tv_nsec / 1000);
 
-	fprintf(stderr, "[%10.3f] %s%s@%u.%s(",
+	fprintf(stderr, "[%10.3f] %s%s%s@%u.%s(",
 		time / 1000.0,
 		send ? " -> " : "",
+		(target->flags & WL_OBJECT_FLAG_INERT) ? "INERT " : "",
 		target->interface->name, target->id,
 		closure->message->name);
 
diff --git a/src/scanner.c b/src/scanner.c
index adc9aa3..e526e50 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -1138,6 +1138,11 @@ emit_messages(struct wl_list *message_list,
 	wl_list_for_each(m, message_list, link) {
 		printf("\t{ \"%s\", \"", m->name);
 
+		/* make destructor flag the first letter in
+		 * signature */
+		if (m->destructor)
+			printf("D");
+
 		if (m->since > 1)
 			printf("%d", m->since);
 
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 92256a1..a3258c4 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -107,6 +107,13 @@ static struct wl_proxy *
 wl_map_lookup_proxy(struct wl_map *objects, uint32_t id)
 {
 	struct wl_object *object = wl_map_lookup(objects, id);
+
+	/* in the case that offset of object in proxy is not 0
+	 * and the proxy is WL_ZOMBIE_OBJECT (which is not embedded
+	 * in any proxy), we could run into troubles. */
+	if (object == WL_ZOMBIE_OBJECT)
+		return (struct wl_proxy *) WL_ZOMBIE_OBJECT;
+
 	return container_of(object, struct wl_proxy, object);
 }
 
diff --git a/src/wayland-private.h b/src/wayland-private.h
index ab48889..5a52d00 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -43,8 +43,20 @@
 #define WL_CLOSURE_MAX_ARGS 20
 
 enum wl_object_flags {
-	WL_OBJECT_FLAG_DESTROYED  = 1 << 0,
-	WL_OBJECT_FLAG_ID_DELETED = 1 << 1
+	/* wl_object has been destroy by a destroy function, but
+	 * lives in references of events/requests */
+	WL_OBJECT_FLAG_DESTROYED	= 1 << 0,
+
+	/* client got delete_id event for this object */
+	WL_OBJECT_FLAG_ID_DELETED	= 1 << 1,
+
+	/* client created this object, but server do not has a
+	 * counter part anymore. This object does not dispatch
+	 * anything but destructors */
+	WL_OBJECT_FLAG_INERT		= 1 << 2,
+
+	/* object is created from inert object */
+	WL_OBJECT_FLAG_INERT_INHERENTLY	= 1 << 3
 };
 
 struct wl_object {
diff --git a/src/wayland-server.c b/src/wayland-server.c
index d34e07c..c1256a3 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -124,6 +124,11 @@ static struct wl_resource *
 wl_map_lookup_resource(struct wl_map *objects, uint32_t id)
 {
 	struct wl_object *object = wl_map_lookup(objects, id);
+
+	/* inherently inert object are not (in any) resource */
+	if (!object || (object->flags & WL_OBJECT_FLAG_INERT_INHERENTLY))
+		return NULL;
+
 	return container_of(object, struct wl_resource, object);
 }
 
@@ -134,6 +139,9 @@ wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode,
 	struct wl_closure *closure;
 	struct wl_object *object = &resource->object;
 
+	if (object->flags & WL_OBJECT_FLAG_INERT)
+		return;
+
 	closure = wl_closure_marshal(object, opcode, args,
 				     &object->interface->events[opcode]);
 
@@ -235,6 +243,15 @@ wl_resource_post_error(struct wl_resource *resource,
 }
 
 static int
+wl_message_is_destructor(const struct wl_message *message)
+{
+	return message->signature[0] == 'D';
+}
+
+static void
+client_delete_id(struct wl_client *client, uint32_t id);
+
+static int
 wl_client_connection_data(int fd, uint32_t mask, void *data)
 {
 	struct wl_client *client = data;
@@ -280,16 +297,18 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 		if (len < size)
 			break;
 
-		resource = wl_map_lookup_resource(&client->objects, p[0]);
 		resource_flags = wl_map_lookup_flags(&client->objects, p[0]);
-		if (resource == NULL) {
+		object = wl_map_lookup(&client->objects, p[0]);
+		if (object == NULL) {
 			wl_resource_post_error(client->display_resource,
 					       WL_DISPLAY_ERROR_INVALID_OBJECT,
 					       "invalid object %u", p[0]);
 			break;
 		}
 
-		object = &resource->object;
+		/* this would be wayland bug */
+		assert(object->interface && "Object without interface");
+
 		if (opcode >= object->interface->method_count) {
 			wl_resource_post_error(client->display_resource,
 					       WL_DISPLAY_ERROR_INVALID_METHOD,
@@ -300,9 +319,18 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 			break;
 		}
 
+		/* inherently inert objects are not embedded in any
+		 * resource. However, we cannot just bail out here.
+		 * We need the closure to be created, because there
+		 * will be possibly created new inherently inert objects */
+		if (object->flags & WL_OBJECT_FLAG_INERT_INHERENTLY)
+			resource = NULL;
+		else
+			resource = wl_container_of(object, resource, object);
+
 		message = &object->interface->methods[opcode];
 		since = wl_message_get_since(message);
-		if (!(resource_flags & WL_MAP_ENTRY_LEGACY) &&
+		if (!(resource_flags & WL_MAP_ENTRY_LEGACY) && resource &&
 		    resource->version > 0 && resource->version < since) {
 			wl_resource_post_error(client->display_resource,
 					       WL_DISPLAY_ERROR_INVALID_METHOD,
@@ -314,13 +342,13 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 			break;
 		}
 
-
 		closure = wl_connection_demarshal(client->connection, size,
 						  &client->objects, message);
 		len -= size;
 
 		if (closure == NULL && errno == ENOMEM) {
-			wl_resource_post_no_memory(resource);
+			if (resource)
+				wl_resource_post_no_memory(resource);
 			break;
 		} else if (closure == NULL ||
 			   wl_closure_lookup_objects(closure, &client->objects) < 0) {
@@ -337,13 +365,48 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 		if (debug_server)
 			wl_closure_print(closure, object, false);
 
-		if ((resource_flags & WL_MAP_ENTRY_LEGACY) ||
-		    resource->dispatcher == NULL) {
-			wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER,
-					  object, opcode, client);
+		/* inert resources dispatch only destructors */
+		if ((object->flags & WL_OBJECT_FLAG_INERT) &&
+		    !wl_message_is_destructor(message)) {
+			/* Let user know why the request "vanished" */
+			wl_log("Skipping non-destructor action on inert resource"
+			       " (%s@%u.%s)\n", object->interface->name, object->id,
+			       message->name);
+			wl_closure_destroy(closure);
+			continue;
+		}
+
+		/* if everything is ok, or if this is destructor of inert object,
+		 * dispatch the request */
+		if (resource) {
+			if ((object->flags & WL_OBJECT_FLAG_INERT)
+			     && !object->implementation) {
+				wl_log("No implementation at inert object %s@%u."
+				       " Blame compositor!\n",
+				       object->interface->name, object->id);
+				wl_closure_destroy(closure);
+				continue;
+			}
+
+			if ((resource_flags & WL_MAP_ENTRY_LEGACY) ||
+			     resource->dispatcher == NULL) {
+				wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER,
+						  object, opcode, client);
+			} else {
+				wl_closure_dispatch(closure, resource->dispatcher,
+						    object, opcode);
+			}
 		} else {
-			wl_closure_dispatch(closure, resource->dispatcher,
-					    object, opcode);
+			/* we are here, but do not have resource => this is a
+			 * destructor of inherently inert object.
+			 * Just send delete_id - destructor is not set anyway */
+			assert(object->flags & WL_OBJECT_FLAG_INERT_INHERENTLY);
+
+			client_delete_id(client, object->id);
+
+			/* inherently inert objects are allocated dynamically.
+			 * We can free them at this moment */
+			free(object);
 		}
 
 		wl_closure_destroy(closure);
@@ -530,9 +593,11 @@ static void
 destroy_resource(void *element, void *data)
 {
 	struct wl_resource *resource = element;
-	struct wl_client *client = resource->client;
+	struct wl_client *client;
 	uint32_t flags;
 
+	client = resource->client;
+
 	wl_signal_emit(&resource->destroy_signal, resource);
 
 	flags = wl_map_lookup_flags(&client->objects, resource->object.id);
@@ -546,19 +611,21 @@ destroy_resource(void *element, void *data)
 static inline void
 destroy_resource_for_object(void *element, void *data)
 {
-	/* element is of type wl_object * */
+	struct wl_object *object = element;
+
+	/* if this is inherently inert object, we cannot destroy
+	 * it as a resource. We just need to free the memory */
+	if (object->flags & WL_OBJECT_FLAG_INERT_INHERENTLY) {
+		free(object);
+		return;
+	}
+
 	destroy_resource(container_of(element, struct wl_resource, object), data);
 }
 
-WL_EXPORT void
-wl_resource_destroy(struct wl_resource *resource)
+static void
+client_delete_id(struct wl_client *client, uint32_t id)
 {
-	struct wl_client *client = resource->client;
-	uint32_t id;
-
-	id = resource->object.id;
-	destroy_resource(resource, NULL);
-
 	if (id < WL_SERVER_ID_START) {
 		if (client->display_resource) {
 			wl_resource_queue_event(client->display_resource,
@@ -570,6 +637,18 @@ wl_resource_destroy(struct wl_resource *resource)
 	}
 }
 
+WL_EXPORT void
+wl_resource_destroy(struct wl_resource *resource)
+{
+	struct wl_client *client = resource->client;
+	uint32_t id;
+
+	id = resource->object.id;
+	destroy_resource(resource, NULL);
+
+	client_delete_id(client, id);
+}
+
 WL_EXPORT uint32_t
 wl_resource_get_id(struct wl_resource *resource)
 {
@@ -1291,6 +1370,18 @@ wl_resource_set_implementation(struct wl_resource *resource,
 }
 
 WL_EXPORT void
+wl_resource_set_inert(struct wl_resource *resource)
+{
+	resource->object.flags |= WL_OBJECT_FLAG_INERT;
+}
+
+WL_EXPORT int
+wl_resource_is_inert(struct wl_resource *resource)
+{
+	return (resource->object.flags & WL_OBJECT_FLAG_INERT);
+}
+
+WL_EXPORT void
 wl_resource_set_dispatcher(struct wl_resource *resource,
 			   wl_dispatcher_func_t dispatcher,
 			   const void *implementation,
@@ -1313,20 +1404,19 @@ wl_resource_create(struct wl_client *client,
 	if (resource == NULL)
 		return NULL;
 
+	memset(resource, 0, sizeof *resource);
+
 	if (id == 0)
 		id = wl_map_insert_new(&client->objects, 0, NULL);
 
 	resource->object.id = id;
 	resource->object.interface = interface;
-	resource->object.implementation = NULL;
+	resource->object.flags = 0;
 
 	wl_signal_init(&resource->destroy_signal);
 
-	resource->destroy = NULL;
 	resource->client = client;
-	resource->data = NULL;
 	resource->version = version;
-	resource->dispatcher = NULL;
 
 	if (wl_map_insert_at(&client->objects, 0, id, &resource->object) < 0) {
 		wl_resource_post_error(client->display_resource,
diff --git a/src/wayland-server.h b/src/wayland-server.h
index af2f03d..41d4c25 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -353,6 +353,12 @@ struct wl_resource *
 wl_resource_create(struct wl_client *client,
 		   const struct wl_interface *interface,
 		   int version, uint32_t id);
+
+void
+wl_resource_set_inert(struct wl_resource *resource);
+int
+wl_resource_is_inert(struct wl_resource *resource);
+
 void
 wl_resource_set_implementation(struct wl_resource *resource,
 			       const void *implementation,
-- 
2.1.0



More information about the wayland-devel mailing list