From ed31ca30cd75d848ed6dcb1f44013c29478b0595 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 4 Jun 2020 17:41:01 +0200 Subject: [PATCH] media-session: improve cleanup of objects Don't try to use the proxy destroy event to destroy the objects. It is not called automatically anymore, only when we call pw_proxy_destroy() ourselves. Destroy the proxy objects when we destroy the session managed object instead, either when the global is removed or when we explicitly call _destroy() Add an object free signal to clean up final resources after the proxies have been destroyed, like closing libraries. Track and destroy our link proxies. --- src/examples/media-session/alsa-monitor.c | 39 ++++--- src/examples/media-session/media-session.c | 117 +++++++++++---------- src/examples/media-session/media-session.h | 1 + src/examples/media-session/v4l2-monitor.c | 27 +++-- 4 files changed, 104 insertions(+), 80 deletions(-) diff --git a/src/examples/media-session/alsa-monitor.c b/src/examples/media-session/alsa-monitor.c index cd246ce69..c794e1b14 100644 --- a/src/examples/media-session/alsa-monitor.c +++ b/src/examples/media-session/alsa-monitor.c @@ -604,9 +604,27 @@ static void device_destroy(void *data) struct node *node; pw_log_debug("device %p destroy", device); + spa_list_remove(&device->link); spa_list_consume(node, &device->node_list, link) alsa_remove_node(device, node); + + if (device->appeared) + spa_hook_remove(&device->device_listener); + if (device->seq != 0) + spa_hook_remove(&device->sync_listener); + if (device->reserve) + rd_device_destroy(device->reserve); +} + +static void device_free(void *data) +{ + struct device *device = data; + pw_log_debug("device %p free", device); + spa_hook_remove(&device->listener); + pw_unload_spa_handle(device->handle); + pw_properties_free(device->props); + free(device); } static void device_update(void *data) @@ -630,8 +648,9 @@ static void device_update(void *data) static const struct sm_object_events device_events = { SM_VERSION_OBJECT_EVENTS, - .destroy = device_destroy, - .update = device_update, + .destroy = device_destroy, + .free = device_free, + .update = device_update, }; static struct device *alsa_create_device(struct impl *impl, uint32_t id, @@ -729,21 +748,9 @@ exit: static void alsa_remove_device(struct impl *impl, struct device *device) { - pw_log_debug("remove device %u", device->id); - spa_list_remove(&device->link); - if (device->appeared) - spa_hook_remove(&device->device_listener); - if (device->seq != 0) - spa_hook_remove(&device->sync_listener); - if (device->reserve) - rd_device_destroy(device->reserve); - if (device->sdevice) { + pw_log_debug("%p: remove device %u", device, device->id); + if (device->sdevice) sm_object_destroy(&device->sdevice->obj); - spa_hook_remove(&device->listener); - } - pw_unload_spa_handle(device->handle); - pw_properties_free(device->props); - free(device); } static void alsa_udev_object_info(void *data, uint32_t id, diff --git a/src/examples/media-session/media-session.c b/src/examples/media-session/media-session.c index 78b4ad0f1..60c03c04a 100644 --- a/src/examples/media-session/media-session.c +++ b/src/examples/media-session/media-session.c @@ -42,6 +42,7 @@ #include #include "pipewire/pipewire.h" +#include "pipewire/private.h" #include "extensions/session-manager.h" #include @@ -54,6 +55,7 @@ #define sm_object_emit_update(s) sm_object_emit(s, update, 0) #define sm_object_emit_destroy(s) sm_object_emit(s, destroy, 0) +#define sm_object_emit_free(s) sm_object_emit(s, free, 0) #define sm_media_session_emit(s,m,v,...) spa_hook_list_call(&(s)->hooks, struct sm_media_session_events, m, v, ##__VA_ARGS__) @@ -237,17 +239,56 @@ int sm_object_remove_data(struct sm_object *obj, const char *id) int sm_object_destroy(struct sm_object *obj) { - pw_log_debug(NAME" %p: object %d", obj->session, obj->id); - if (obj->proxy) { - pw_proxy_destroy(obj->proxy); - if (obj->handle == obj->proxy) - obj->handle = NULL; - obj->proxy = NULL; + struct impl *impl = SPA_CONTAINER_OF(obj->session, struct impl, this); + struct data *d; + struct pw_proxy *p, *h; + + p = obj->proxy; + h = obj->handle; + + pw_log_debug(NAME" %p: object %d proxy:%p handle:%p", obj->session, + obj->id, p, h); + + sm_object_emit_destroy(obj); + + if (SPA_FLAG_IS_SET(obj->mask, SM_OBJECT_CHANGE_MASK_LISTENER)) { + SPA_FLAG_CLEAR(obj->mask, SM_OBJECT_CHANGE_MASK_LISTENER); + spa_hook_remove(&obj->object_listener); } - if (obj->handle) { - pw_proxy_destroy(obj->handle); - obj->handle = NULL; + + if (obj->id != SPA_ID_INVALID) + remove_object(impl, obj); + + if (obj->destroy) + obj->destroy(obj); + + if (p) { + pw_proxy_ref(p); + spa_hook_remove(&obj->proxy_listener); } + if (h) { + pw_proxy_ref(h); + spa_hook_remove(&obj->handle_listener); + } + if (p) + pw_proxy_destroy(p); + if (h != p) + pw_proxy_destroy(h); + + sm_object_emit_free(obj); + + if (obj->props) + pw_properties_free(obj->props); + obj->props = NULL; + + spa_list_consume(d, &obj->data, link) { + spa_list_remove(&d->link); + free(d); + } + if (p) + pw_proxy_unref(p); + if (h) + pw_proxy_unref(h); return 0; } @@ -911,46 +952,6 @@ static const struct object_info endpoint_link_info = { /** * Proxy */ -static void -destroy_proxy(void *data) -{ - struct sm_object *obj = data; - struct impl *impl = SPA_CONTAINER_OF(obj->session, struct impl, this); - struct data *d; - - pw_log_debug("object %p: proxy:%p id:%d", obj, obj->proxy, obj->id); - - if (SPA_FLAG_IS_SET(obj->mask, SM_OBJECT_CHANGE_MASK_LISTENER)) { - SPA_FLAG_CLEAR(obj->mask, SM_OBJECT_CHANGE_MASK_LISTENER); - spa_hook_remove(&obj->object_listener); - } - - if (obj->id != SPA_ID_INVALID) - remove_object(impl, obj); - - sm_object_emit_destroy(obj); - - if (obj->destroy) - obj->destroy(obj); - - if (obj->proxy) { - spa_hook_remove(&obj->proxy_listener); - obj->proxy = NULL; - } - if (obj->handle) { - spa_hook_remove(&obj->handle_listener); - obj->handle = NULL; - } - if (obj->props) - pw_properties_free(obj->props); - obj->props = NULL; - - spa_list_consume(d, &obj->data, link) { - spa_list_remove(&d->link); - free(d); - } -} - static void done_proxy(void *data, int seq) { struct sm_object *obj = data; @@ -980,7 +981,6 @@ static void bound_proxy(void *data, uint32_t id) static const struct pw_proxy_events proxy_events = { PW_VERSION_PROXY_EVENTS, - .destroy = destroy_proxy, .done = done_proxy, .bound = bound_proxy, }; @@ -1074,7 +1074,8 @@ create_object(struct impl *impl, struct pw_proxy *proxy, struct pw_proxy *handle } obj = init_object(impl, info, proxy, handle, SPA_ID_INVALID, props); - pw_log_debug(NAME" %p: created new object %p proxy %p", impl, obj, obj->proxy); + pw_log_debug(NAME" %p: created new object %p proxy:%p handle:%p", impl, + obj, obj->proxy, obj->handle); return obj; } @@ -1271,7 +1272,7 @@ registry_global_remove(void *data, uint32_t id) if ((obj = find_object(impl, id)) == NULL) return; - remove_object(impl, obj); + sm_object_destroy(obj); } static const struct pw_registry_events registry_events = { @@ -1391,6 +1392,12 @@ static void check_endpoint_link(struct endpoint_link *link) } } +static void proxy_link_removed(void *data) +{ + struct link *l = data; + pw_proxy_destroy(l->proxy); +} + static void proxy_link_destroy(void *data) { struct link *l = data; @@ -1404,6 +1411,7 @@ static void proxy_link_destroy(void *data) static const struct pw_proxy_events proxy_link_events = { PW_VERSION_PROXY_EVENTS, + .removed = proxy_link_removed, .destroy = proxy_link_destroy }; @@ -1695,8 +1703,8 @@ static void session_shutdown(struct impl *impl) pw_log_info(NAME" %p", impl); - spa_list_for_each(obj, &impl->global_list, link) - sm_media_session_emit_remove(impl, obj); + spa_list_consume(obj, &impl->global_list, link) + sm_object_destroy(obj); sm_media_session_emit_destroy(impl); @@ -1887,6 +1895,7 @@ exit: pw_map_clear(&impl.endpoint_links); pw_map_clear(&impl.globals); + pw_properties_free(impl.this.props); pw_deinit(); diff --git a/src/examples/media-session/media-session.h b/src/examples/media-session/media-session.h index fc3b9aef7..f619e4693 100644 --- a/src/examples/media-session/media-session.h +++ b/src/examples/media-session/media-session.h @@ -43,6 +43,7 @@ struct sm_object_events { void (*update) (void *data); void (*destroy) (void *data); + void (*free) (void *data); }; struct sm_object_methods { diff --git a/src/examples/media-session/v4l2-monitor.c b/src/examples/media-session/v4l2-monitor.c index 61637e551..3f75887a2 100644 --- a/src/examples/media-session/v4l2-monitor.c +++ b/src/examples/media-session/v4l2-monitor.c @@ -295,9 +295,23 @@ static void device_destroy(void *data) struct node *node; pw_log_debug("device %p destroy", device); + spa_list_remove(&device->link); spa_list_consume(node, &device->node_list, link) v4l2_remove_node(device, node); + + if (device->appeared) + spa_hook_remove(&device->device_listener); +} + +static void device_free(void *data) +{ + struct device *device = data; + pw_log_debug("device %p free", device); + spa_hook_remove(&device->listener); + pw_unload_spa_handle(device->handle); + pw_properties_free(device->props); + free(device); } static void device_update(void *data) @@ -322,11 +336,11 @@ static void device_update(void *data) static const struct sm_object_events device_events = { SM_VERSION_OBJECT_EVENTS, - .destroy = device_destroy, - .update = device_update, + .destroy = device_destroy, + .free = device_free, + .update = device_update, }; - static struct device *v4l2_create_device(struct impl *impl, uint32_t id, const struct spa_device_object_info *info) { @@ -401,15 +415,8 @@ exit: static void v4l2_remove_device(struct impl *impl, struct device *dev) { pw_log_debug("remove device %u", dev->id); - spa_list_remove(&dev->link); - if (dev->appeared) - spa_hook_remove(&dev->device_listener); if (dev->sdevice) sm_object_destroy(&dev->sdevice->obj); - spa_hook_remove(&dev->listener); - pw_unload_spa_handle(dev->handle); - pw_properties_free(dev->props); - free(dev); } static void v4l2_udev_object_info(void *data, uint32_t id,