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

@@ -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)

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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)

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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)
+static void on_module_loaded(void *data, int result)
-{
-	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)
 {
 	struct load_module_data *d = data;
 	struct module *module = d->module;
+	struct client *client = d->client;
 	struct impl *impl = module->impl;
-	struct message *reply;
+	uint32_t tag = d->tag;
-	struct client *client;
-	uint32_t tag;
 
-	client = d->client;
+	spa_hook_remove(&d->listener);
-	tag = d->tag;
+	free(d);
-	load_module_data_free(d);
 
-	if (error < 0) {
+	if (SPA_RESULT_IS_OK(result)) {
-		pw_log_warn(NAME" %p: [%s] error loading module", client->impl, client->name);
+		struct message *reply;
-		reply_error(client, COMMAND_LOAD_MODULE, tag, error);
-		return;
-	}
 
-	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);
+		reply = reply_new(client, tag);
-	message_put(reply,
+		message_put(reply,
-		TAG_U32, module->idx,
+			TAG_U32, module->idx,
-		TAG_INVALID);
+			TAG_INVALID);
-	send_message(client, reply);
+		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;
+	static const struct module_events listener = {
-	struct impl *impl = client->impl;
-	struct load_module_data *d;
-	const char *name, *argument;
-	static 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)