protocol-native: improve client and resource refcounts

Add a refcount to resource and client so that we can keep them alive
while the native protocol is using them.

One problem might be that the protocol destroys the client or resource
while handling it and that would cause errors.

Fixes #565

src/modules/module-protocol-native.c

@@ -347,9 +347,12 @@ process_messages(struct client_data *data)
 			continue;
 		}
 
+		resource->refcount++;
 		pw_protocol_native_connection_enter(conn);
 		res = demarshal[msg->opcode].func(resource, msg);
 		pw_protocol_native_connection_leave(conn);
+		pw_resource_unref(resource);
+
 		if (res < 0) {
 			pw_resource_errorf_id(resource, msg->id,
 					res, "invalid message id:%u op:%u (%s)",
@@ -360,6 +363,7 @@ process_messages(struct client_data *data)
 	res = 0;
 done:
 	context->current_client = NULL;
+
 	return res;
 
 error:
@@ -387,13 +391,14 @@ client_busy_changed(void *data, bool busy)
 		pw_loop_signal_event(s->loop, s->resume);
 }
 
-static void handle_client_error(struct pw_impl_client *client, int res)
+static void handle_client_error(struct pw_impl_client *client, int res, const char *msg)
 {
 	if (res == -EPIPE || res == -ECONNRESET)
-		pw_log_info("%p: client %p disconnected", client->protocol, client);
+		pw_log_info("%p: %s: client %p disconnected", client->protocol, msg, client);
 	else
-		pw_log_error("%p: client %p error %d (%s)", client->protocol,
+		pw_log_error("%p: %s: client %p error %d (%s)", client->protocol, msg,
 				client, res, spa_strerror(res));
+	if (!client->destroyed)
 		pw_impl_client_destroy(client);
 }
 
@@ -404,6 +409,8 @@ connection_data(void *data, int fd, uint32_t mask)
 	struct pw_impl_client *client = this->client;
 	int res;
 
+	client->refcount++;
+
 	if (mask & SPA_IO_HUP) {
 		res = -EPIPE;
 		goto error;
@@ -425,9 +432,12 @@ connection_data(void *data, int fd, uint32_t mask)
 		} else if (res != -EAGAIN)
 			goto error;
 	}
+done:
+	pw_impl_client_unref(client);
 	return;
 error:
-	handle_client_error(client, res);
+	handle_client_error(client, res, "connection_data");
+	goto done;
 }
 
 static void client_free(void *data)
@@ -1215,12 +1225,12 @@ static void do_resume(void *_data, uint64_t count)
 	pw_log_debug("flush");
 
 	spa_list_for_each_safe(data, tmp, &this->client_list, protocol_link) {
+		data->client->refcount++;
 		if ((res = process_messages(data)) < 0)
-			goto error;
+			handle_client_error(data->client, res, "do_resume");
+		pw_impl_client_unref(data->client);
 	}
 	return;
-error:
-	handle_client_error(data->client, res);
 }
 
 static const char *

src/pipewire/impl-client.c

@@ -24,6 +24,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <assert.h>
 
 #include <spa/utils/string.h>
 
@@ -415,6 +416,7 @@ struct pw_impl_client *pw_context_create_client(struct pw_impl_core *core,
 	this = &impl->this;
 	pw_log_debug("%p: new", this);
 
+	this->refcount = 1;
 	this->context = core->context;
 	this->core = core;
 	this->protocol = protocol;
@@ -584,6 +586,33 @@ static int destroy_resource(void *object, void *data)
 }
 
 
+SPA_EXPORT
+void pw_impl_client_unref(struct pw_impl_client *client)
+{
+	struct impl *impl = SPA_CONTAINER_OF(client, struct impl, this);
+
+	assert(client->refcount > 0);
+	if (--client->refcount > 0)
+		return;
+
+	pw_log_debug("%p: free", impl);
+	assert(client->destroyed);
+
+	pw_impl_client_emit_free(client);
+
+	spa_hook_list_clean(&client->listener_list);
+
+	pw_map_clear(&client->objects);
+	pw_array_clear(&impl->permissions);
+
+	spa_hook_remove(&impl->pool_listener);
+	pw_mempool_destroy(client->pool);
+
+	pw_properties_free(client->properties);
+
+	free(impl);
+}
+
 /** Destroy a client object
  *
  * \param client the client to destroy
@@ -595,6 +624,10 @@ void pw_impl_client_destroy(struct pw_impl_client *client)
 	struct impl *impl = SPA_CONTAINER_OF(client, struct impl, this);
 
 	pw_log_debug("%p: destroy", client);
+
+	assert(!client->destroyed);
+	client->destroyed = true;
+
 	pw_impl_client_emit_destroy(client);
 
 	spa_hook_remove(&impl->context_listener);
@@ -609,20 +642,7 @@ void pw_impl_client_destroy(struct pw_impl_client *client)
 		pw_global_destroy(client->global);
 	}
 
-	pw_log_debug("%p: free", impl);
-	pw_impl_client_emit_free(client);
-
-	spa_hook_list_clean(&client->listener_list);
-
-	pw_map_clear(&client->objects);
-	pw_array_clear(&impl->permissions);
-
-	spa_hook_remove(&impl->pool_listener);
-	pw_mempool_destroy(client->pool);
-
-	pw_properties_free(client->properties);
-
-	free(impl);
+	pw_impl_client_unref(client);
 }
 
 SPA_EXPORT

src/pipewire/private.h

@@ -308,6 +308,9 @@ struct pw_impl_client {
 	unsigned int registered:1;
 	unsigned int ucred_valid:1;	/**< if the ucred member is valid */
 	unsigned int busy:1;
+	unsigned int destroyed:1;
+
+	int refcount;
 
 	/* v2 compatibility data */
 	void *compat_v2;
@@ -945,8 +948,10 @@ struct pw_resource {
 	const char *type;		/**< type of the client interface */
 	uint32_t version;		/**< version of the client interface */
 	uint32_t bound_id;		/**< global id we are bound to */
+	int refcount;
 
 	unsigned int removed:1;		/**< resource was removed from server */
+	unsigned int destroyed:1;	/**< resource was destroyed */
 
 	struct spa_hook_list listener_list;
 	struct spa_hook_list object_listener_list;
@@ -1266,6 +1271,8 @@ int pw_control_remove_link(struct pw_control_link *link);
 
 void pw_control_destroy(struct pw_control *control);
 
+void pw_impl_client_unref(struct pw_impl_client *client);
+
 #define PW_LOG_OBJECT_POD	(1<<0)
 void pw_log_log_object(enum spa_log_level level, const char *file, int line,
 	   const char *func, uint32_t flags, const void *object);

src/pipewire/resource.c

@@ -23,6 +23,7 @@
  */
 
 #include <string.h>
+#include <assert.h>
 
 #include "pipewire/private.h"
 #include "pipewire/protocol.h"
@@ -57,6 +58,7 @@ struct pw_resource *pw_resource_new(struct pw_impl_client *client,
 		return NULL;
 
 	this = &impl->this;
+	this->refcount = 1;
 	this->context = client->context;
 	this->client = client;
 	this->permissions = permissions;
@@ -278,17 +280,58 @@ void pw_resource_error(struct pw_resource *resource, int res, const char *error)
 	}
 }
 
+SPA_EXPORT
+void pw_resource_ref(struct pw_resource *resource)
+{
+	assert(resource->refcount > 0);
+	resource->refcount++;
+}
+
+SPA_EXPORT
+void pw_resource_unref(struct pw_resource *resource)
+{
+	assert(resource->refcount > 0);
+	if (--resource->refcount > 0)
+		return;
+
+	pw_log_debug("%p: free %u", resource, resource->id);
+	assert(resource->destroyed);
+
+#if DEBUG_LISTENERS
+	{
+		struct spa_hook *h;
+		spa_list_for_each(h, &resource->object_listener_list.list, link) {
+			pw_log_warn("%p: resource %u: leaked object listener %p",
+					resource, resource->id, h);
+			break;
+		}
+		spa_list_for_each(h, &resource->listener_list.list, link) {
+			pw_log_warn("%p: resource %u: leaked listener %p",
+					resource, resource->id, h);
+			break;
+		}
+	}
+#endif
+	spa_hook_list_clean(&resource->listener_list);
+	spa_hook_list_clean(&resource->object_listener_list);
+
+	free(resource);
+}
+
 SPA_EXPORT
 void pw_resource_destroy(struct pw_resource *resource)
 {
 	struct pw_impl_client *client = resource->client;
 
+	pw_log_debug("%p: destroy %u", resource, resource->id);
+	assert(!resource->destroyed);
+	resource->destroyed = true;
+
 	if (resource->global) {
 		spa_list_remove(&resource->link);
 		resource->global = NULL;
 	}
 
-	pw_log_debug("%p: destroy %u", resource, resource->id);
 	pw_resource_emit_destroy(resource);
 
 	pw_map_insert_at(&client->objects, resource->id, NULL);
@@ -297,12 +340,7 @@ void pw_resource_destroy(struct pw_resource *resource)
 	if (client->core_resource && !resource->removed)
 		pw_core_resource_remove_id(client->core_resource, resource->id);
 
-	pw_log_debug("%p: free %u", resource, resource->id);
-
-	spa_hook_list_clean(&resource->listener_list);
-	spa_hook_list_clean(&resource->object_listener_list);
-
-	free(resource);
+	pw_resource_unref(resource);
 }
 
 SPA_EXPORT

src/pipewire/resource.h

@@ -121,6 +121,10 @@ void pw_resource_add_object_listener(struct pw_resource *resource,
  * with the same \a sequence number in the return value. */
 int pw_resource_ping(struct pw_resource *resource, int seq);
 
+/** ref/unref a resource, Since 0.3.52 */
+void pw_resource_ref(struct pw_resource *resource);
+void pw_resource_unref(struct pw_resource *resource);
+
 /** Notify global id this resource is bound to */
 int pw_resource_set_bound_id(struct pw_resource *resource, uint32_t global_id);