From bd6f63fecda384ccb895fb70a805456b135f8afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 10 Jun 2021 18:37:48 +0200 Subject: [PATCH] pulse-server: improve module loading Modules no longer need to emit the "loaded" event manually if they can load immediately. In that case, the module loading code will take care of emitting the event. If they can't, they must return an async spa result, and emit the "loaded" event when they see fit. Fixes #1232 --- src/modules/module-protocol-pulse/module.h | 2 +- .../modules/module-combine-sink.c | 2 - .../modules/module-echo-cancel.c | 2 - .../modules/module-ladspa-sink.c | 2 - .../modules/module-ladspa-source.c | 2 - .../modules/module-loopback.c | 2 - .../modules/module-native-protocol-tcp.c | 2 - .../modules/module-null-sink.c | 2 +- .../modules/module-pipe-sink.c | 2 - .../modules/module-remap-sink.c | 2 - .../modules/module-remap-source.c | 2 - .../modules/module-simple-protocol-tcp.c | 2 - .../modules/module-tunnel-sink.c | 2 - .../modules/module-tunnel-source.c | 2 - .../modules/module-zeroconf-discover.c | 2 - .../module-protocol-pulse/pulse-server.c | 97 +++++++++++-------- 16 files changed, 57 insertions(+), 70 deletions(-) diff --git a/src/modules/module-protocol-pulse/module.h b/src/modules/module-protocol-pulse/module.h index 442044b90..b4260bbcd 100644 --- a/src/modules/module-protocol-pulse/module.h +++ b/src/modules/module-protocol-pulse/module.h @@ -41,7 +41,7 @@ struct module_events { #define VERSION_MODULE_EVENTS 0 uint32_t version; - void (*loaded) (void *data, int res); + void (*loaded) (void *data, int result); }; #define module_emit_loaded(m,r) spa_hook_list_call(&m->listener_list, struct module_events, loaded, 0, r) diff --git a/src/modules/module-protocol-pulse/modules/module-combine-sink.c b/src/modules/module-protocol-pulse/modules/module-combine-sink.c index 32b15c06f..4b3a2c3e2 100644 --- a/src/modules/module-protocol-pulse/modules/module-combine-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-combine-sink.c @@ -361,8 +361,6 @@ static int module_combine_sink_load(struct client *client, struct module *module data->cleanup = pw_loop_add_event(module->impl->loop, on_cleanup, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-echo-cancel.c b/src/modules/module-protocol-pulse/modules/module-echo-cancel.c index 5c9a7adf6..913a6882d 100644 --- a/src/modules/module-protocol-pulse/modules/module-echo-cancel.c +++ b/src/modules/module-protocol-pulse/modules/module-echo-cancel.c @@ -107,8 +107,6 @@ static int module_echo_cancel_load(struct client *client, struct module *module) &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-ladspa-sink.c b/src/modules/module-protocol-pulse/modules/module-ladspa-sink.c index e35d4305b..3485a8786 100644 --- a/src/modules/module-protocol-pulse/modules/module-ladspa-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-ladspa-sink.c @@ -110,8 +110,6 @@ static int module_ladspa_sink_load(struct client *client, struct module *module) &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-ladspa-source.c b/src/modules/module-protocol-pulse/modules/module-ladspa-source.c index fb50e97a7..13497446a 100644 --- a/src/modules/module-protocol-pulse/modules/module-ladspa-source.c +++ b/src/modules/module-protocol-pulse/modules/module-ladspa-source.c @@ -110,8 +110,6 @@ static int module_ladspa_source_load(struct client *client, struct module *modul &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-loopback.c b/src/modules/module-protocol-pulse/modules/module-loopback.c index ba64a4588..1bc0c13a6 100644 --- a/src/modules/module-protocol-pulse/modules/module-loopback.c +++ b/src/modules/module-protocol-pulse/modules/module-loopback.c @@ -98,8 +98,6 @@ static int module_loopback_load(struct client *client, struct module *module) &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c b/src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c index eae8a4e9a..bd89f79ea 100644 --- a/src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c +++ b/src/modules/module-protocol-pulse/modules/module-native-protocol-tcp.c @@ -56,8 +56,6 @@ static int module_native_protocol_tcp_load(struct client *client, struct module if (res < 0) return res; - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-null-sink.c b/src/modules/module-protocol-pulse/modules/module-null-sink.c index 9bd7ecfd2..c53fa4e3a 100644 --- a/src/modules/module-protocol-pulse/modules/module-null-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-null-sink.c @@ -91,7 +91,7 @@ static int module_null_sink_load(struct client *client, struct module *module) pw_proxy_add_listener(d->proxy, &d->listener, &proxy_events, module); - return 0; + return SPA_RESULT_RETURN_ASYNC(0); } static int module_null_sink_unload(struct client *client, struct module *module) diff --git a/src/modules/module-protocol-pulse/modules/module-pipe-sink.c b/src/modules/module-protocol-pulse/modules/module-pipe-sink.c index d6f213a54..fe6ab4955 100644 --- a/src/modules/module-protocol-pulse/modules/module-pipe-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-pipe-sink.c @@ -177,8 +177,6 @@ static int module_pipesink_load(struct client *client, struct module *module) params, n_params)) < 0) return res; - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-remap-sink.c b/src/modules/module-protocol-pulse/modules/module-remap-sink.c index cdc5409a8..0661fdbf6 100644 --- a/src/modules/module-protocol-pulse/modules/module-remap-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-remap-sink.c @@ -94,8 +94,6 @@ static int module_remap_sink_load(struct client *client, struct module *module) &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-remap-source.c b/src/modules/module-protocol-pulse/modules/module-remap-source.c index 03c6a62c2..1979ed223 100644 --- a/src/modules/module-protocol-pulse/modules/module-remap-source.c +++ b/src/modules/module-protocol-pulse/modules/module-remap-source.c @@ -94,8 +94,6 @@ static int module_remap_source_load(struct client *client, struct module *module &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-simple-protocol-tcp.c b/src/modules/module-protocol-pulse/modules/module-simple-protocol-tcp.c index e3459e467..eab5ace09 100644 --- a/src/modules/module-protocol-pulse/modules/module-simple-protocol-tcp.c +++ b/src/modules/module-protocol-pulse/modules/module-simple-protocol-tcp.c @@ -89,8 +89,6 @@ static int module_simple_protocol_tcp_load(struct client *client, struct module pw_impl_module_add_listener(data->mod, &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-tunnel-sink.c b/src/modules/module-protocol-pulse/modules/module-tunnel-sink.c index 688251440..804e4e831 100644 --- a/src/modules/module-protocol-pulse/modules/module-tunnel-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-tunnel-sink.c @@ -95,8 +95,6 @@ static int module_tunnel_sink_load(struct client *client, struct module *module) &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-tunnel-source.c b/src/modules/module-protocol-pulse/modules/module-tunnel-source.c index 3cf1cc4bd..b622c1986 100644 --- a/src/modules/module-protocol-pulse/modules/module-tunnel-source.c +++ b/src/modules/module-protocol-pulse/modules/module-tunnel-source.c @@ -95,8 +95,6 @@ static int module_tunnel_source_load(struct client *client, struct module *modul &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/modules/module-zeroconf-discover.c b/src/modules/module-protocol-pulse/modules/module-zeroconf-discover.c index e64329eef..8191b8978 100644 --- a/src/modules/module-protocol-pulse/modules/module-zeroconf-discover.c +++ b/src/modules/module-protocol-pulse/modules/module-zeroconf-discover.c @@ -71,8 +71,6 @@ static int module_zeroconf_discover_load(struct client *client, struct module *m &data->mod_listener, &module_events, data); - module_emit_loaded(module, 0); - return 0; } diff --git a/src/modules/module-protocol-pulse/pulse-server.c b/src/modules/module-protocol-pulse/pulse-server.c index 843af72aa..38b6d4249 100644 --- a/src/modules/module-protocol-pulse/pulse-server.c +++ b/src/modules/module-protocol-pulse/pulse-server.c @@ -123,6 +123,7 @@ static void broadcast_subscribe_event(struct impl *impl, uint32_t mask, uint32_t #include "message-handler.c" static void client_free(struct client *client); +static void client_unref(struct client *client); static void sample_free(struct sample *sample) { @@ -4975,73 +4976,69 @@ static int do_kill(struct client *client, uint32_t command, uint32_t tag, struct } struct load_module_data { - struct spa_list link; struct client *client; struct module *module; struct spa_hook listener; uint32_t tag; }; -static struct load_module_data *load_module_data_new(struct client *client, uint32_t tag) -{ - struct load_module_data *data = calloc(1, sizeof(struct load_module_data)); - data->client = client; - data->tag = tag; - return data; -} - -static void load_module_data_free(struct load_module_data *d) -{ - spa_hook_remove(&d->listener); - free(d); -} - -static void on_module_loaded(void *data, int error) +static void on_module_loaded(void *data, int result) { struct load_module_data *d = data; struct module *module = d->module; + struct client *client = d->client; struct impl *impl = module->impl; - struct message *reply; - struct client *client; - uint32_t tag; + uint32_t tag = d->tag; - client = d->client; - tag = d->tag; - load_module_data_free(d); + spa_hook_remove(&d->listener); + free(d); - if (error < 0) { - pw_log_warn(NAME" %p: [%s] error loading module", client->impl, client->name); - reply_error(client, COMMAND_LOAD_MODULE, tag, error); - return; - } + if (SPA_RESULT_IS_OK(result)) { + struct message *reply; - pw_log_info(NAME" %p: [%s] module %d loaded", client->impl, client->name, module->idx); + pw_log_info(NAME" %p: [%s] loaded module id:%u name:%s", + impl, client->name, + module->idx, module->name); - module->loaded = true; + module->loaded = true; - broadcast_subscribe_event(impl, + broadcast_subscribe_event(impl, SUBSCRIPTION_MASK_MODULE, SUBSCRIPTION_EVENT_NEW | SUBSCRIPTION_EVENT_MODULE, module->idx); - reply = reply_new(client, tag); - message_put(reply, - TAG_U32, module->idx, - TAG_INVALID); - send_message(client, reply); + reply = reply_new(client, tag); + message_put(reply, + TAG_U32, module->idx, + TAG_INVALID); + send_message(client, reply); + } + else { + pw_log_warn(NAME" %p: [%s] failed to load module id:%u name:%s result:%d (%s)", + impl, client->name, + module->idx, module->name, + result, spa_strerror(result)); + + reply_error(client, COMMAND_LOAD_MODULE, tag, result); + module_schedule_unload(module); + } + + client_unref(client); } static int do_load_module(struct client *client, uint32_t command, uint32_t tag, struct message *m) { - struct module *module; - struct impl *impl = client->impl; - struct load_module_data *d; - const char *name, *argument; - static struct module_events listener = { + static const struct module_events listener = { VERSION_MODULE_EVENTS, .loaded = on_module_loaded, }; + struct impl *impl = client->impl; + const char *name, *argument; + struct load_module_data *d; + struct module *module; + int r; + if (message_get(m, TAG_STRING, &name, TAG_STRING, &argument, @@ -5055,11 +5052,27 @@ static int do_load_module(struct client *client, uint32_t command, uint32_t tag, if (module == NULL) return -errno; - d = load_module_data_new(client, tag); + d = calloc(1, sizeof(*d)); + if (d == NULL) + return -errno; + + d->tag = tag; + d->client = client; d->module = module; module_add_listener(module, &d->listener, &listener, d); - return module_load(client, module); + client->ref += 1; + + r = module_load(client, module); + if (!SPA_RESULT_IS_ASYNC(r)) + module_emit_loaded(module, r); + /* in the async case the module itself must emit the "loaded" event */ + + /* + * return 0 to prevent `handle_packet()` from sending a reply + * because we want `on_module_loaded()` to send the reply + */ + return 0; } static int do_unload_module(struct client *client, uint32_t command, uint32_t tag, struct message *m)