diff --git a/src/modules/module-protocol-native.c b/src/modules/module-protocol-native.c index 3295ed4fc..6869f5c1d 100644 --- a/src/modules/module-protocol-native.c +++ b/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,14 +391,15 @@ 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)); - pw_impl_client_destroy(client); + if (!client->destroyed) + pw_impl_client_destroy(client); } static void @@ -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 * diff --git a/src/pipewire/impl-client.c b/src/pipewire/impl-client.c index cd566a566..f45e3827f 100644 --- a/src/pipewire/impl-client.c +++ b/src/pipewire/impl-client.c @@ -24,6 +24,7 @@ #include #include +#include #include @@ -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 diff --git a/src/pipewire/private.h b/src/pipewire/private.h index 05c42f6c7..52629f4a3 100644 --- a/src/pipewire/private.h +++ b/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); diff --git a/src/pipewire/resource.c b/src/pipewire/resource.c index efb82a41e..0a6231c2e 100644 --- a/src/pipewire/resource.c +++ b/src/pipewire/resource.c @@ -23,6 +23,7 @@ */ #include +#include #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 diff --git a/src/pipewire/resource.h b/src/pipewire/resource.h index dd15c1a8f..24d458cb0 100644 --- a/src/pipewire/resource.h +++ b/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);