From eeda53dbefce5f3b077d8603644034ec9ee63fe0 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 8 Aug 2017 18:22:44 +0200 Subject: [PATCH] Fix leaks Add some docs Add properties to loop objects for future use --- spa/include/spa/hook.h | 17 ++++++++++++-- spa/plugins/support/loop.c | 1 + src/daemon/main.c | 4 ++-- src/examples/export-sink.c | 2 +- src/examples/export-v4l2.c | 2 +- src/examples/local-v4l2.c | 2 +- src/examples/video-play.c | 5 +++-- src/examples/video-src.c | 2 +- src/extensions/client-node.h | 7 ++++++ src/gst/gstpipewiredeviceprovider.c | 6 +++-- src/gst/gstpipewiresink.c | 2 +- src/gst/gstpipewiresrc.c | 2 +- src/modules/module-client-node/transport.c | 26 ++++++++++------------ src/modules/module-client-node/transport.h | 3 --- src/pipewire/core.c | 19 ++++++++++++---- src/pipewire/data-loop.c | 4 ++-- src/pipewire/data-loop.h | 2 +- src/pipewire/loop.c | 2 +- src/pipewire/loop.h | 3 ++- src/pipewire/main-loop.c | 4 ++-- src/pipewire/main-loop.h | 2 +- src/pipewire/remote.c | 7 ++++-- src/pipewire/stream.c | 16 ++++++++----- src/pipewire/thread-loop.c | 3 ++- src/tools/pipewire-monitor.c | 2 +- 25 files changed, 92 insertions(+), 53 deletions(-) diff --git a/spa/include/spa/hook.h b/spa/include/spa/hook.h index 177216525..96248a21f 100644 --- a/spa/include/spa/hook.h +++ b/spa/include/spa/hook.h @@ -26,22 +26,32 @@ extern "C" { #include +/** \class spa_hook + * + * \brief a list of hooks + * + * The hook list provides a way to keep track of hooks. + */ +/** A list of hooks */ struct spa_hook_list { struct spa_list list; }; +/** A hook, contains the structure with functions and the data passed + * to the functions. */ struct spa_hook { struct spa_list link; const void *funcs; void *data; }; +/** Initialize a hook list */ static inline void spa_hook_list_init(struct spa_hook_list *list) { spa_list_init(&list->list); } -/** Add a hook \memberof spa_hook */ +/** Append a hook \memberof spa_hook */ static inline void spa_hook_list_append(struct spa_hook_list *list, struct spa_hook *hook, const void *funcs, void *data) @@ -51,6 +61,7 @@ static inline void spa_hook_list_append(struct spa_hook_list *list, spa_list_append(&list->list, &hook->link); } +/** Prepend a hook \memberof spa_hook */ static inline void spa_hook_list_prepend(struct spa_hook_list *list, struct spa_hook *hook, const void *funcs, void *data) @@ -60,12 +71,14 @@ static inline void spa_hook_list_prepend(struct spa_hook_list *list, spa_list_prepend(&list->list, &hook->link); } -/** Remove a listener \memberof spa_hook */ +/** Remove a hook \memberof spa_hook */ static inline void spa_hook_remove(struct spa_hook *hook) { spa_list_remove(&hook->link); } +/** Call all hooks in a list, starting from the given one and optionally stopping + * after calling the first non-NULL function */ #define spa_hook_list_do_call(l,start,type,method,once,...) ({ \ struct spa_hook_list *list = l; \ struct spa_list *s = start ? (struct spa_list *)start : &list->list; \ diff --git a/spa/plugins/support/loop.c b/spa/plugins/support/loop.c index 11487dd76..0ac25238a 100644 --- a/spa/plugins/support/loop.c +++ b/spa/plugins/support/loop.c @@ -686,6 +686,7 @@ static int impl_clear(struct spa_handle *handle) spa_list_for_each_safe(source, tmp, &impl->destroy_list, link) free(source); + close(impl->ack_fd); close(impl->epoll_fd); return SPA_RESULT_OK; diff --git a/src/daemon/main.c b/src/daemon/main.c index 1f5ec4f1e..3566f9c39 100644 --- a/src/daemon/main.c +++ b/src/daemon/main.c @@ -41,11 +41,11 @@ int main(int argc, char *argv[]) return -1; } - loop = pw_main_loop_new(); - props = pw_properties_new("pipewire.core.name", "pipewire-0", "pipewire.daemon", "1", NULL); + loop = pw_main_loop_new(props); + core = pw_core_new(pw_main_loop_get_loop(loop), props); pw_daemon_config_run_commands(config, core); diff --git a/src/examples/export-sink.c b/src/examples/export-sink.c index befbd83a0..dac782df8 100644 --- a/src/examples/export-sink.c +++ b/src/examples/export-sink.c @@ -473,7 +473,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.t = pw_core_get_type(data.core); diff --git a/src/examples/export-v4l2.c b/src/examples/export-v4l2.c index 27ec2114f..6afd06690 100644 --- a/src/examples/export-v4l2.c +++ b/src/examples/export-v4l2.c @@ -117,7 +117,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.t = pw_core_get_type(data.core); diff --git a/src/examples/local-v4l2.c b/src/examples/local-v4l2.c index ac94c3822..595180e14 100644 --- a/src/examples/local-v4l2.c +++ b/src/examples/local-v4l2.c @@ -437,7 +437,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.t = pw_core_get_type(data.core); diff --git a/src/examples/video-play.c b/src/examples/video-play.c index 4e77632ba..430bd7736 100644 --- a/src/examples/video-play.c +++ b/src/examples/video-play.c @@ -91,7 +91,7 @@ static void handle_events(struct data *data) while (SDL_PollEvent(&event)) { switch (event.type) { case SDL_QUIT: - exit(0); + data->running = false; break; } } @@ -385,7 +385,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.t = pw_core_get_type(data.core); @@ -418,6 +418,7 @@ int main(int argc, char *argv[]) pw_loop_leave(data.loop); pw_remote_destroy(data.remote); + pw_core_destroy(data.core); pw_loop_destroy(data.loop); return 0; diff --git a/src/examples/video-src.c b/src/examples/video-src.c index 78a471176..77c6f36f9 100644 --- a/src/examples/video-src.c +++ b/src/examples/video-src.c @@ -281,7 +281,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.t = pw_core_get_type(data.core); diff --git a/src/extensions/client-node.h b/src/extensions/client-node.h index dd0436e6d..f39af559f 100644 --- a/src/extensions/client-node.h +++ b/src/extensions/client-node.h @@ -65,6 +65,12 @@ struct pw_client_node_transport { void *output_data; /**< output memory for ringbuffer */ struct spa_ringbuffer *output_buffer; /**< ringbuffer for output memory */ + /** Destroy a transport + * \param trans a transport to destroy + * \memberof pw_client_node_transport + */ + void (*destroy) (struct pw_client_node_transport *trans); + /** Add a message to the transport * \param trans the transport to send the message on * \param message the message to add @@ -98,6 +104,7 @@ struct pw_client_node_transport { int (*parse_message) (struct pw_client_node_transport *trans, void *message); }; +#define pw_client_node_transport_destroy(t) ((t)->destroy((t))) #define pw_client_node_transport_add_message(t,m) ((t)->add_message((t), (m))) #define pw_client_node_transport_next_message(t,m) ((t)->next_message((t), (m))) #define pw_client_node_transport_parse_message(t,m) ((t)->parse_message((t), (m))) diff --git a/src/gst/gstpipewiredeviceprovider.c b/src/gst/gstpipewiredeviceprovider.c index 252d97a8f..d9bbc9d2a 100644 --- a/src/gst/gstpipewiredeviceprovider.c +++ b/src/gst/gstpipewiredeviceprovider.c @@ -426,7 +426,7 @@ gst_pipewire_device_provider_probe (GstDeviceProvider * provider) GST_DEBUG_OBJECT (self, "starting probe"); - if (!(l = pw_loop_new ())) + if (!(l = pw_loop_new (NULL))) return NULL; if (!(c = pw_core_new (l, NULL))) @@ -503,7 +503,7 @@ gst_pipewire_device_provider_start (GstDeviceProvider * provider) GST_DEBUG_OBJECT (self, "starting provider"); - self->loop = pw_loop_new (); + self->loop = pw_loop_new (NULL); self->list_only = FALSE; if (!(self->main_loop = pw_thread_loop_new (self->loop, "pipewire-device-monitor"))) { @@ -590,6 +590,8 @@ gst_pipewire_device_provider_stop (GstDeviceProvider * provider) { GstPipeWireDeviceProvider *self = GST_PIPEWIRE_DEVICE_PROVIDER (provider); + GST_DEBUG_OBJECT (self, "stopping provider"); + if (self->remote) { pw_remote_disconnect (self->remote); pw_remote_destroy (self->remote); diff --git a/src/gst/gstpipewiresink.c b/src/gst/gstpipewiresink.c index eb6b1f2c0..654b7469e 100644 --- a/src/gst/gstpipewiresink.c +++ b/src/gst/gstpipewiresink.c @@ -302,7 +302,7 @@ gst_pipewire_sink_init (GstPipeWireSink * sink) g_queue_init (&sink->queue); - sink->loop = pw_loop_new (); + sink->loop = pw_loop_new (NULL); sink->main_loop = pw_thread_loop_new (sink->loop, "pipewire-sink-loop"); sink->core = pw_core_new (sink->loop, NULL); sink->type = pw_core_get_type (sink->core); diff --git a/src/gst/gstpipewiresrc.c b/src/gst/gstpipewiresrc.c index ee1e857f7..bee909934 100644 --- a/src/gst/gstpipewiresrc.c +++ b/src/gst/gstpipewiresrc.c @@ -308,7 +308,7 @@ gst_pipewire_src_init (GstPipeWireSrc * src) src->client_name = pw_get_client_name (); src->buf_ids = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, (GDestroyNotify) gst_buffer_unref); - src->loop = pw_loop_new (); + src->loop = pw_loop_new (NULL); src->main_loop = pw_thread_loop_new (src->loop, "pipewire-main-loop"); src->core = pw_core_new (src->loop, NULL); src->type = pw_core_get_type (src->core); diff --git a/src/modules/module-client-node/transport.c b/src/modules/module-client-node/transport.c index b2c815029..06afd780e 100644 --- a/src/modules/module-client-node/transport.c +++ b/src/modules/module-client-node/transport.c @@ -98,6 +98,16 @@ static void transport_reset_area(struct pw_client_node_transport *trans) spa_ringbuffer_init(trans->output_buffer, OUTPUT_BUFFER_SIZE); } +static void destroy(struct pw_client_node_transport *trans) +{ + struct transport *impl = (struct transport *) trans; + + pw_log_debug("transport %p: destroy", trans); + + pw_memblock_free(&impl->mem); + free(impl); +} + static int add_message(struct pw_client_node_transport *trans, struct pw_client_node_message *message) { struct transport *impl = (struct transport *) trans; @@ -194,6 +204,7 @@ pw_client_node_transport_new(uint32_t max_input_ports, uint32_t max_output_ports transport_setup_area(impl->mem.ptr, trans); transport_reset_area(trans); + trans->destroy = destroy; trans->add_message = add_message; trans->next_message = next_message; trans->parse_message = parse_message; @@ -236,6 +247,7 @@ pw_client_node_transport_new_from_info(struct pw_client_node_transport_info *inf trans->output_data = trans->input_data; trans->input_data = tmp; + trans->destroy = destroy; trans->add_message = add_message; trans->next_message = next_message; trans->parse_message = parse_message; @@ -247,20 +259,6 @@ pw_client_node_transport_new_from_info(struct pw_client_node_transport_info *inf return NULL; } -/** Destroy a transport - * \param trans a transport to destroy - * \memberof pw_client_node_transport - */ -void pw_client_node_transport_destroy(struct pw_client_node_transport *trans) -{ - struct transport *impl = (struct transport *) trans; - - pw_log_debug("transport %p: destroy", trans); - - pw_memblock_free(&impl->mem); - free(impl); -} - /** Get transport info * \param trans the transport to get info of * \param[out] info transport info diff --git a/src/modules/module-client-node/transport.h b/src/modules/module-client-node/transport.h index d1af562f0..c0c014737 100644 --- a/src/modules/module-client-node/transport.h +++ b/src/modules/module-client-node/transport.h @@ -44,9 +44,6 @@ pw_client_node_transport_new(uint32_t max_input_ports, uint32_t max_output_ports struct pw_client_node_transport * pw_client_node_transport_new_from_info(struct pw_client_node_transport_info *info); -void -pw_client_node_transport_destroy(struct pw_client_node_transport *trans); - int pw_client_node_transport_get_info(struct pw_client_node_transport *trans, struct pw_client_node_transport_info *info); diff --git a/src/pipewire/core.c b/src/pipewire/core.c index 470458022..e517805b8 100644 --- a/src/pipewire/core.c +++ b/src/pipewire/core.c @@ -347,7 +347,7 @@ struct pw_core *pw_core_new(struct pw_loop *main_loop, struct pw_properties *pro if (this == NULL) return NULL; - this->data_loop_impl = pw_data_loop_new(); + this->data_loop_impl = pw_data_loop_new(properties); if (this->data_loop_impl == NULL) goto no_data_loop; @@ -400,9 +400,13 @@ struct pw_core *pw_core_new(struct pw_loop *main_loop, struct pw_properties *pro this->info.name = pw_properties_get(properties, "pipewire.core.name"); this->properties = properties; - this->global = pw_core_add_global(this, NULL, NULL, this->type.core, PW_VERSION_CORE, - core_bind_func, this); - + this->global = pw_core_add_global(this, + NULL, + NULL, + this->type.core, + PW_VERSION_CORE, + core_bind_func, + this); return this; no_data_loop: @@ -418,11 +422,18 @@ struct pw_core *pw_core_new(struct pw_loop *main_loop, struct pw_properties *pro */ void pw_core_destroy(struct pw_core *core) { + struct pw_global *global, *t; + pw_log_debug("core %p: destroy", core); spa_hook_list_call(&core->listener_list, struct pw_core_events, destroy, core); + spa_list_for_each_safe(global, t, &core->global_list, link) + pw_global_destroy(global); + pw_data_loop_destroy(core->data_loop_impl); + pw_properties_free(core->properties); + pw_map_clear(&core->globals); pw_log_debug("core %p: free", core); diff --git a/src/pipewire/data-loop.c b/src/pipewire/data-loop.c index 480c87dd6..346c58d98 100644 --- a/src/pipewire/data-loop.c +++ b/src/pipewire/data-loop.c @@ -101,7 +101,7 @@ static void do_stop(struct spa_loop_utils *utils, struct spa_source *source, uin * * \memberof pw_data_loop */ -struct pw_data_loop *pw_data_loop_new(void) +struct pw_data_loop *pw_data_loop_new(struct pw_properties *properties) { struct pw_data_loop *this; @@ -111,7 +111,7 @@ struct pw_data_loop *pw_data_loop_new(void) pw_log_debug("data-loop %p: new", this); - this->loop = pw_loop_new(); + this->loop = pw_loop_new(properties); if (this->loop == NULL) goto no_loop; diff --git a/src/pipewire/data-loop.h b/src/pipewire/data-loop.h index 7a47d471e..c1f047b65 100644 --- a/src/pipewire/data-loop.h +++ b/src/pipewire/data-loop.h @@ -40,7 +40,7 @@ struct pw_data_loop_events { }; struct pw_data_loop * -pw_data_loop_new(void); +pw_data_loop_new(struct pw_properties *properties); void pw_data_loop_add_listener(struct pw_data_loop *loop, struct spa_hook *listener, diff --git a/src/pipewire/loop.c b/src/pipewire/loop.c index 0bd5cd9dc..5ebce5977 100644 --- a/src/pipewire/loop.c +++ b/src/pipewire/loop.c @@ -41,7 +41,7 @@ struct impl { * \returns a newly allocated loop * \memberof pw_loop */ -struct pw_loop *pw_loop_new(void) +struct pw_loop *pw_loop_new(struct pw_properties *properties) { int res; struct impl *impl; diff --git a/src/pipewire/loop.h b/src/pipewire/loop.h index 9a4cd6ac0..866bae738 100644 --- a/src/pipewire/loop.h +++ b/src/pipewire/loop.h @@ -26,6 +26,7 @@ extern "C" { #include +#include /** \class pw_loop * @@ -40,7 +41,7 @@ struct pw_loop { }; struct pw_loop * -pw_loop_new(void); +pw_loop_new(struct pw_properties *properties); void pw_loop_destroy(struct pw_loop *loop); diff --git a/src/pipewire/main-loop.c b/src/pipewire/main-loop.c index 65a35f8d7..0d085423a 100644 --- a/src/pipewire/main-loop.c +++ b/src/pipewire/main-loop.c @@ -26,7 +26,7 @@ * * \memberof pw_main_loop */ -struct pw_main_loop *pw_main_loop_new(void) +struct pw_main_loop *pw_main_loop_new(struct pw_properties *properties) { struct pw_main_loop *this; @@ -36,7 +36,7 @@ struct pw_main_loop *pw_main_loop_new(void) pw_log_debug("main-loop %p: new", this); - this->loop = pw_loop_new(); + this->loop = pw_loop_new(properties); if (this->loop == NULL) goto no_loop; diff --git a/src/pipewire/main-loop.h b/src/pipewire/main-loop.h index 9c1df7675..b701baed0 100644 --- a/src/pipewire/main-loop.h +++ b/src/pipewire/main-loop.h @@ -44,7 +44,7 @@ struct pw_main_loop_events { }; struct pw_main_loop * -pw_main_loop_new(void); +pw_main_loop_new(struct pw_properties *properties); void pw_main_loop_add_listener(struct pw_main_loop *loop, struct spa_hook *listener, diff --git a/src/pipewire/remote.c b/src/pipewire/remote.c index 7007e9f1d..2a43553fd 100644 --- a/src/pipewire/remote.c +++ b/src/pipewire/remote.c @@ -271,7 +271,7 @@ void pw_remote_destroy(struct pw_remote *remote) pw_remote_disconnect(remote); spa_list_for_each_safe(stream, s2, &remote->stream_list, link) - pw_stream_destroy(stream); + pw_stream_destroy(stream); pw_protocol_connection_destroy (remote->conn); @@ -1005,7 +1005,10 @@ struct pw_proxy *pw_remote_export(struct pw_remote *remote, pw_node_add_listener(node, &data->node_listener, &node_events, data); - pw_client_node_proxy_add_listener(data->node_proxy, &data->proxy_listener, &client_node_events, proxy); + pw_client_node_proxy_add_listener(data->node_proxy, + &data->proxy_listener, + &client_node_events, + proxy); do_node_init(proxy); return proxy; diff --git a/src/pipewire/stream.c b/src/pipewire/stream.c index 825d9d72d..446db4035 100644 --- a/src/pipewire/stream.c +++ b/src/pipewire/stream.c @@ -260,7 +260,6 @@ do_remove_sources(struct spa_loop *loop, struct stream *impl = user_data; struct pw_stream *stream = &impl->this; - printf("removing sources\n"); if (impl->rtsocket_source) { pw_loop_destroy_source(stream->remote->core->data_loop, impl->rtsocket_source); impl->rtsocket_source = NULL; @@ -328,13 +327,13 @@ void pw_stream_destroy(struct pw_stream *stream) spa_hook_list_call(&stream->listener_list, struct pw_stream_events, destroy); - unhandle_socket(stream); - - spa_list_remove(&stream->link); - if (impl->node_proxy) spa_hook_remove(&impl->proxy_listener); + pw_stream_disconnect(stream); + + spa_list_remove(&stream->link); + set_possible_formats(stream, 0, NULL); set_params(stream, 0, NULL); @@ -872,6 +871,8 @@ static void client_node_transport(void *data, uint32_t node_id, stream->node_id = node_id; + if (impl->trans) + pw_client_node_transport_destroy(impl->trans); impl->trans = transport; pw_log_info("stream %p: create client transport %p with fds %d %d for node %u", @@ -903,7 +904,6 @@ static void on_node_proxy_destroy(void *data) impl->disconnecting = false; impl->node_proxy = NULL; - spa_hook_remove(&impl->proxy_listener); stream_set_state(this, PW_STREAM_STATE_UNCONNECTED, NULL); @@ -994,6 +994,10 @@ void pw_stream_disconnect(struct pw_stream *stream) pw_client_node_proxy_destroy(impl->node_proxy); impl->node_proxy = NULL; } + if (impl->trans) { + pw_client_node_transport_destroy(impl->trans); + impl->trans = NULL; + } } bool pw_stream_get_time(struct pw_stream *stream, struct pw_time *time) diff --git a/src/pipewire/thread-loop.c b/src/pipewire/thread-loop.c index b22c8fc2a..97932db16 100644 --- a/src/pipewire/thread-loop.c +++ b/src/pipewire/thread-loop.c @@ -83,7 +83,8 @@ static void do_stop(struct spa_loop_utils *utils, struct spa_source *source, uin * * \memberof pw_thread_loop */ -struct pw_thread_loop *pw_thread_loop_new(struct pw_loop *loop, const char *name) +struct pw_thread_loop *pw_thread_loop_new(struct pw_loop *loop, + const char *name) { struct pw_thread_loop *this; pthread_mutexattr_t attr; diff --git a/src/tools/pipewire-monitor.c b/src/tools/pipewire-monitor.c index d8e0cc76e..48bada793 100644 --- a/src/tools/pipewire-monitor.c +++ b/src/tools/pipewire-monitor.c @@ -385,7 +385,7 @@ int main(int argc, char *argv[]) pw_init(&argc, &argv); - data.loop = pw_loop_new(); + data.loop = pw_loop_new(NULL); data.running = true; data.core = pw_core_new(data.loop, NULL); data.remote = pw_remote_new(data.core, NULL);