bluez5: properly handle dbus signals

Fixes pipewire/pipewire#502.

Also move media application creating from filter callback to initialization.
Application object path and endpoint paths only need to be registered once.

Signed-off-by: Huang-Huang Bao <eh5@sokka.cn>

spa/plugins/bluez5/backend-hsp-native.c

@@ -574,9 +574,6 @@ static void register_profile_reply(DBusPendingCall *pending, void *user_data)
 
 static int register_profile(struct spa_bt_backend *backend, const char *profile, const char *uuid)
 {
-	static const DBusObjectPathVTable vtable_profile = {
-		.message_function = profile_handler,
-	};
 	DBusMessage *m;
 	DBusMessageIter it[4];
 	dbus_bool_t autoconnect;
@@ -586,11 +583,6 @@ static int register_profile(struct spa_bt_backend *backend, const char *profile,
 
 	spa_log_debug(backend->log, NAME": Registering Profile %s %s", profile, uuid);
 
-	if (!dbus_connection_register_object_path(backend->conn,
-						  profile,
-						  &vtable_profile, backend))
-		return -EIO;
-
 	m = dbus_message_new_method_call(BLUEZ_SERVICE, "/org/bluez",
 			BLUEZ_PROFILE_MANAGER_INTERFACE, "RegisterProfile");
 	if (m == NULL)
@@ -649,6 +641,8 @@ void backend_hsp_native_register_profiles(struct spa_bt_backend *backend)
 
 void backend_hsp_native_free(struct spa_bt_backend *backend)
 {
+	dbus_connection_unregister_object_path(backend->conn, PROFILE_HSP_AG);
+	dbus_connection_unregister_object_path(backend->conn, PROFILE_HSP_HS);
 	free(backend);
 }
 
@@ -658,6 +652,9 @@ struct spa_bt_backend *backend_hsp_native_new(struct spa_bt_monitor *monitor,
 	  uint32_t n_support)
 {
 	struct spa_bt_backend *backend;
+	static const DBusObjectPathVTable vtable_profile = {
+		.message_function = profile_handler,
+	};
 
 	backend = calloc(1, sizeof(struct spa_bt_backend));
 	if (backend == NULL)
@@ -669,5 +666,20 @@ struct spa_bt_backend *backend_hsp_native_new(struct spa_bt_monitor *monitor,
 	backend->main_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Loop);
 	backend->conn = dbus_connection;
 
+	if (!dbus_connection_register_object_path(backend->conn,
+						  PROFILE_HSP_AG,
+						  &vtable_profile, backend)) {
+		goto fail;
+	}
+
+	if (!dbus_connection_register_object_path(backend->conn,
+						  PROFILE_HSP_HS,
+						  &vtable_profile, backend)) {
+		goto fail;
+	}
+
 	return backend;
+fail:
+	free(backend);
+	return NULL;
 }

spa/plugins/bluez5/bluez5-dbus.c

@@ -73,6 +73,7 @@ struct spa_bt_monitor {
 	struct spa_list transport_list;
 
 	unsigned int filters_added:1;
+	unsigned int objects_listed:1;
 
 	struct spa_bt_backend *backend_hsp_native;
 	struct spa_bt_backend *backend_ofono;
@@ -85,6 +86,16 @@ static inline void add_dict(struct spa_pod_builder *builder, const char *key, co
 	spa_pod_builder_string(builder, val);
 }
 
+static int a2dp_codec_to_endpoint(const struct a2dp_codec *codec,
+				   const char * endpoint,
+				   char** object_path)
+{
+	*object_path = spa_aprintf("%s/%s", endpoint, codec->name);
+	if (*object_path == NULL)
+		return -errno;
+	return 0;
+}
+
 static const struct a2dp_codec *a2dp_endpoint_to_codec(const char *endpoint)
 {
 	const char *codec_name;
@@ -1194,16 +1205,20 @@ static void append_basic_array_variant_dict_entry(DBusMessageIter *dict, int key
 }
 
 static int bluez_register_endpoint(struct spa_bt_monitor *monitor,
-                             const char *path, const char *uuid,
+                             const char *path, const char *endpoint,
-                             const struct a2dp_codec *codec, const char *object_path) {
+			     const char *uuid, const struct a2dp_codec *codec) {
-	char* str;
+	char *str, *object_path = NULL;
 	DBusMessage *m;
 	DBusMessageIter object_it, dict_it;
 	DBusPendingCall *call;
 	uint8_t caps[A2DP_MAX_CAPS_SIZE];
-	int caps_size;
+	int ret, caps_size;
 	uint16_t codec_id = codec->codec_id;
 
+	ret = a2dp_codec_to_endpoint(codec, endpoint, &object_path);
+	if (ret < 0)
+		return ret;
+
 	caps_size = codec->fill_caps(codec, 0, caps);
 	if (caps_size < 0)
 		return caps_size;
@@ -1233,34 +1248,41 @@ static int bluez_register_endpoint(struct spa_bt_monitor *monitor,
 	dbus_pending_call_set_notify(call, bluez_register_endpoint_reply, monitor, NULL);
 	dbus_message_unref(m);
 
+	free(object_path);
+
 	return 0;
 }
 
 static int register_a2dp_endpoint(struct spa_bt_monitor *monitor,
-		const struct a2dp_codec *codec, const char *endpoint, char** object_path)
+		const struct a2dp_codec *codec, const char *endpoint)
 {
+	int ret;
+	char* object_path = NULL;
 	const DBusObjectPathVTable vtable_endpoint = {
 		.message_function = endpoint_handler,
 	};
 
-	*object_path = spa_aprintf("%s/%s", endpoint, codec->name);
+	ret = a2dp_codec_to_endpoint(codec, endpoint, &object_path);
-	if (*object_path == NULL)
+	if (ret < 0)
-		return -errno;
+		return ret;
 
-	spa_log_info(monitor->log, "Registering endpoint: %s", *object_path);
+	spa_log_info(monitor->log, "Registering endpoint: %s", object_path);
 
 	if (!dbus_connection_register_object_path(monitor->conn,
-	                                          *object_path,
+	                                          object_path,
-	                                          &vtable_endpoint, monitor))
+	                                          &vtable_endpoint, monitor)) {
+		free(object_path);
 		return -EIO;
+	}
 
+	free(object_path);
 	return 0;
+
 }
 
 static int adapter_register_endpoints(struct spa_bt_adapter *a)
 {
 	struct spa_bt_monitor *monitor = a->monitor;
-	char *endpoint_path = NULL;
 	int i;
 	int err = 0;
 
@@ -1282,22 +1304,16 @@ static int adapter_register_endpoints(struct spa_bt_adapter *a)
 		if (codec->codec_id != A2DP_CODEC_SBC)
 			continue;
 
-		if ((err = register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT, &endpoint_path)))
-			goto out;
 		if ((err = bluez_register_endpoint(monitor, a->path,
+						   A2DP_SOURCE_ENDPOINT,
 		                                   SPA_BT_UUID_A2DP_SOURCE,
-		                                   codec,
+		                                   codec)))
-		                                   endpoint_path)))
 			goto out;
-		free(endpoint_path);
-		endpoint_path = NULL;
 
-		if ((err = register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT, &endpoint_path)))
-			goto out;
 		if ((err = bluez_register_endpoint(monitor, a->path,
+						   A2DP_SINK_ENDPOINT,
 		                                   SPA_BT_UUID_A2DP_SINK,
-		                                   codec,
+		                                   codec)))
-		                                   endpoint_path)))
 			goto out;
 
 		a->endpoints_registered = true;
@@ -1314,8 +1330,6 @@ static int adapter_register_endpoints(struct spa_bt_adapter *a)
 	if (err) {
 		spa_log_error(monitor->log, "Failed to register bluez5 endpoints");
 	}
-	if (endpoint_path)
-		free(endpoint_path);
 	return err;
 }
 
@@ -1388,7 +1402,7 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage *
 		for (i = 0; a2dp_codecs[i]; i++) {
 			const struct a2dp_codec *codec = a2dp_codecs[i];
 			uint8_t caps[A2DP_MAX_CAPS_SIZE];
-			int caps_size;
+			int caps_size, ret;
 			uint16_t codec_id = codec->codec_id;
 
 			caps_size = codec->fill_caps(codec, 0, caps);
@@ -1396,18 +1410,23 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage *
 				continue;
 
 			if (codec->decode != NULL) {
-				spa_log_info(monitor->log, "register A2DP codec %s", a2dp_codecs[i]->name);
+				ret = a2dp_codec_to_endpoint(codec, A2DP_SINK_ENDPOINT, &endpoint);
-				endpoint = spa_aprintf("%s/%s", A2DP_SINK_ENDPOINT, codec->name);
+				if (ret == 0) {
-				append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SINK,
+					spa_log_info(monitor->log, "register A2DP sink codec %s: %s", a2dp_codecs[i]->name, endpoint);
-						codec_id, caps, caps_size);
+					append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SINK,
-				free(endpoint);
+							codec_id, caps, caps_size);
+					free(endpoint);
+				}
 			}
 
 			if (codec->encode != NULL) {
-				endpoint = spa_aprintf("%s/%s", A2DP_SOURCE_ENDPOINT, codec->name);
+				ret = a2dp_codec_to_endpoint(codec, A2DP_SOURCE_ENDPOINT, &endpoint);
-				append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SOURCE,
+				if (ret == 0) {
-						codec_id, caps, caps_size);
+					spa_log_info(monitor->log, "register A2DP source codec %s: %s", a2dp_codecs[i]->name, endpoint);
-				free(endpoint);
+					append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SOURCE,
+							codec_id, caps, caps_size);
+					free(endpoint);
+				}
 			}
 		}
 
@@ -1424,12 +1443,10 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage *
 
 static void bluez_register_application_reply(DBusPendingCall *pending, void *user_data)
 {
-	char *endpoint_path;
 	struct spa_bt_adapter *adapter = user_data;
 	struct spa_bt_monitor *monitor = adapter->monitor;
 	DBusMessage *r;
 	bool fallback = true;
-	int i;
 
 	r = dbus_pending_call_steal_reply(pending);
 	if (r == NULL)
@@ -1448,20 +1465,8 @@ static void bluez_register_application_reply(DBusPendingCall *pending, void *use
 
 	fallback = false;
 	adapter->application_registered = true;
-	/* The application was registered in bluez5 - time to register the endpoints */
-	for (i = 0; a2dp_codecs[i]; i++) {
-		const struct a2dp_codec *codec = a2dp_codecs[i];
 
-		register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT, &endpoint_path);
+finish:
-		if (endpoint_path)
-			free(endpoint_path);
-
-		register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT, &endpoint_path);
-		if (endpoint_path)
-			free(endpoint_path);
-	}
-
-	finish:
 	dbus_message_unref(r);
 	dbus_pending_call_unref(pending);
 
@@ -1469,12 +1474,54 @@ static void bluez_register_application_reply(DBusPendingCall *pending, void *use
 		adapter_register_endpoints(adapter);
 }
 
-static int adapter_register_application(struct spa_bt_adapter *a) {
+static int register_media_application(struct spa_bt_monitor * monitor)
-	const char *object_manager_path = A2DP_OBJECT_MANAGER_PATH;
+{
-	struct spa_bt_monitor *monitor = a->monitor;
 	const DBusObjectPathVTable vtable_object_manager = {
 		.message_function = object_manager_handler,
 	};
+
+	spa_log_info(monitor->log, "Registering media application object: " A2DP_OBJECT_MANAGER_PATH);
+
+	if (!dbus_connection_register_object_path(monitor->conn,
+	                                          A2DP_OBJECT_MANAGER_PATH,
+	                                          &vtable_object_manager, monitor))
+		return -EIO;
+
+	for (int i = 0; a2dp_codecs[i]; i++) {
+		const struct a2dp_codec *codec = a2dp_codecs[i];
+		register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT);
+		register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT);
+	}
+
+	return 0;
+}
+
+static void unregister_media_application(struct spa_bt_monitor * monitor)
+{
+	int ret;
+	char *object_path = NULL;
+	dbus_connection_unregister_object_path(monitor->conn, A2DP_OBJECT_MANAGER_PATH);
+
+	for (int i = 0; a2dp_codecs[i]; i++) {
+		const struct a2dp_codec *codec = a2dp_codecs[i];
+
+		ret = a2dp_codec_to_endpoint(codec, A2DP_SOURCE_ENDPOINT, &object_path);
+		if (ret == 0) {
+			dbus_connection_unregister_object_path(monitor->conn, object_path);
+			free(object_path);
+		}
+
+		ret = a2dp_codec_to_endpoint(codec, A2DP_SINK_ENDPOINT, &object_path);
+		if (ret == 0) {
+			dbus_connection_unregister_object_path(monitor->conn, object_path);
+			free(object_path);
+		}
+	}
+}
+
+static int adapter_register_application(struct spa_bt_adapter *a) {
+	const char *object_manager_path = A2DP_OBJECT_MANAGER_PATH;
+	struct spa_bt_monitor *monitor = a->monitor;
 	DBusMessage *m;
 	DBusMessageIter i, d;
 	DBusPendingCall *call;
@@ -1484,19 +1531,12 @@ static int adapter_register_application(struct spa_bt_adapter *a) {
 
 	spa_log_debug(monitor->log, "Registering bluez5 media application on adapter %s", a->path);
 
-	if (!dbus_connection_register_object_path(monitor->conn,
-	                                          object_manager_path,
-	                                          &vtable_object_manager, monitor))
-	return -EIO;
-
 	m = dbus_message_new_method_call(BLUEZ_SERVICE,
 	                                 a->path,
 	                                 BLUEZ_MEDIA_INTERFACE,
 	                                 "RegisterApplication");
-	if (m == NULL) {
+	if (m == NULL)
-		dbus_connection_unregister_object_path(monitor->conn, object_manager_path);
 		return -EIO;
-	}
 
 	dbus_message_iter_init_append(m, &i);
 	dbus_message_iter_append_basic(&i, DBUS_TYPE_OBJECT_PATH, &object_manager_path);
@@ -1551,6 +1591,60 @@ static void interface_added(struct spa_bt_monitor *monitor,
 	}
 }
 
+static void interfaces_added(struct spa_bt_monitor *monitor, DBusMessageIter *arg_iter)
+{
+	DBusMessageIter it[3];
+	const char *object_path;
+
+	dbus_message_iter_get_basic(arg_iter, &object_path);
+	dbus_message_iter_next(arg_iter);
+	dbus_message_iter_recurse(arg_iter, &it[0]);
+
+	while (dbus_message_iter_get_arg_type(&it[0]) != DBUS_TYPE_INVALID) {
+		const char *interface_name;
+
+		dbus_message_iter_recurse(&it[0], &it[1]);
+		dbus_message_iter_get_basic(&it[1], &interface_name);
+		dbus_message_iter_next(&it[1]);
+		dbus_message_iter_recurse(&it[1], &it[2]);
+
+		interface_added(monitor, monitor->conn,
+				object_path, interface_name,
+				&it[2]);
+
+		dbus_message_iter_next(&it[0]);
+	}
+}
+
+static void interfaces_removed(struct spa_bt_monitor *monitor, DBusMessageIter *arg_iter)
+{
+	const char *object_path;
+	DBusMessageIter it;
+
+	dbus_message_iter_get_basic(arg_iter, &object_path);
+	dbus_message_iter_next(arg_iter);
+	dbus_message_iter_recurse(arg_iter, &it);
+
+	while (dbus_message_iter_get_arg_type(&it) != DBUS_TYPE_INVALID) {
+		const char *interface_name;
+
+		dbus_message_iter_get_basic(&it, &interface_name);
+
+		if (strcmp(interface_name, BLUEZ_DEVICE_INTERFACE) == 0) {
+			struct spa_bt_device *d;
+			spa_list_consume(d, &monitor->device_list, link)
+				device_free(d);
+		} else if (strcmp(interface_name, BLUEZ_ADAPTER_INTERFACE) == 0) {
+			struct spa_bt_adapter *a;
+			spa_list_consume(a, &monitor->adapter_list, link) {
+				adapter_free(a);
+			}
+		}
+
+		dbus_message_iter_next(&it);
+	}
+}
+
 static void get_managed_objects_reply(DBusPendingCall *pending, void *user_data)
 {
 	struct spa_bt_monitor *monitor = user_data;
@@ -1581,27 +1675,10 @@ static void get_managed_objects_reply(DBusPendingCall *pending, void *user_data)
 	dbus_message_iter_recurse(&it[0], &it[1]);
 
 	while (dbus_message_iter_get_arg_type(&it[1]) != DBUS_TYPE_INVALID) {
-		const char *object_path;
-
 		dbus_message_iter_recurse(&it[1], &it[2]);
-		dbus_message_iter_get_basic(&it[2], &object_path);
-		dbus_message_iter_next(&it[2]);
-		dbus_message_iter_recurse(&it[2], &it[3]);
 
-		while (dbus_message_iter_get_arg_type(&it[3]) != DBUS_TYPE_INVALID) {
+		interfaces_added(monitor, &it[2]);
-			const char *interface_name;
 
-			dbus_message_iter_recurse(&it[3], &it[4]);
-			dbus_message_iter_get_basic(&it[4], &interface_name);
-			dbus_message_iter_next(&it[4]);
-			dbus_message_iter_recurse(&it[4], &it[5]);
-
-			interface_added(monitor, monitor->conn,
-					object_path, interface_name,
-					&it[5]);
-
-			dbus_message_iter_next(&it[3]);
-		}
 		dbus_message_iter_next(&it[1]);
 	}
 
@@ -1634,11 +1711,69 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 	dbus_error_init(&err);
 
 	if (dbus_message_is_signal(m, "org.freedesktop.DBus", "NameOwnerChanged")) {
+		const char *name, *old_owner, *new_owner;
+
 		spa_log_debug(monitor->log, "Name owner changed %s", dbus_message_get_path(m));
+
+		if (!dbus_message_get_args(m, &err,
+					   DBUS_TYPE_STRING, &name,
+					   DBUS_TYPE_STRING, &old_owner,
+					   DBUS_TYPE_STRING, &new_owner,
+					   DBUS_TYPE_INVALID)) {
+			spa_log_error(monitor->log, NAME": Failed to parse org.freedesktop.DBus.NameOwnerChanged: %s", err.message);
+			goto fail;
+		}
+
+		if (strcmp(name, BLUEZ_SERVICE) == 0) {
+			if (old_owner && *old_owner) {
+				struct spa_bt_adapter *a;
+				struct spa_bt_device *d;
+				struct spa_bt_transport *t;
+
+				spa_log_debug(monitor->log, "Bluetooth daemon disappeared");
+				monitor->objects_listed = false;
+
+				spa_list_consume(t, &monitor->transport_list, link)
+					spa_bt_transport_free(t);
+				spa_list_consume(d, &monitor->device_list, link)
+					device_free(d);
+				spa_list_consume(a, &monitor->adapter_list, link)
+					adapter_free(a);
+			}
+
+			if (new_owner && *new_owner) {
+				spa_log_debug(monitor->log, "Bluetooth daemon appeared");
+				get_managed_objects(monitor);
+			}
+		}
 	} else if (dbus_message_is_signal(m, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded")) {
+		DBusMessageIter it;
+
 		spa_log_debug(monitor->log, "interfaces added %s", dbus_message_get_path(m));
+
+		if (!monitor->objects_listed)
+			goto finish;
+
+		if (!dbus_message_iter_init(m, &it) || strcmp(dbus_message_get_signature(m), "oa{sa{sv}}") != 0) {
+			spa_log_error(monitor->log, NAME": Invalid signature found in InterfacesAdded");
+			goto finish;
+		}
+
+		interfaces_added(monitor, &it);
 	} else if (dbus_message_is_signal(m, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved")) {
+		DBusMessageIter it;
+
 		spa_log_debug(monitor->log, "interfaces removed %s", dbus_message_get_path(m));
+
+		if (!monitor->objects_listed)
+			goto finish;
+
+		if (!dbus_message_iter_init(m, &it) || strcmp(dbus_message_get_signature(m), "oas") != 0) {
+			spa_log_error(monitor->log, NAME": Invalid signature found in InterfacesRemoved");
+			goto finish;
+		}
+
+		interfaces_removed(monitor, &it);
 	} else if (dbus_message_is_signal(m, "org.freedesktop.DBus.Properties", "PropertiesChanged")) {
 		DBusMessageIter it[2];
 		const char *iface, *path;
@@ -1646,7 +1781,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 		if (!dbus_message_iter_init(m, &it[0]) ||
 		    strcmp(dbus_message_get_signature(m), "sa{sv}as") != 0) {
 			spa_log_error(monitor->log, "Invalid signature found in PropertiesChanged");
-			goto fail;
+			goto finish;
 		}
 		path = dbus_message_get_path(m);
 
@@ -1661,7 +1796,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 			if (a == NULL) {
 				spa_log_warn(monitor->log,
 						"Properties changed in unknown adapter %s", path);
-				goto fail;
+				goto finish;
 			}
 			spa_log_debug(monitor->log, "Properties changed in adapter %s", path);
 
@@ -1674,7 +1809,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 			if (d == NULL) {
 				spa_log_debug(monitor->log,
 						"Properties changed in unknown device %s", path);
-				goto fail;
+				goto finish;
 			}
 			spa_log_debug(monitor->log, "Properties changed in device %s", path);
 
@@ -1687,7 +1822,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 			if (transport == NULL) {
 				spa_log_warn(monitor->log,
 						"Properties changed in unknown transport %s", path);
-				goto fail;
+				goto finish;
 			}
 
 			spa_log_debug(monitor->log, "Properties changed in transport %s", path);
@@ -1697,6 +1832,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
         }
 
 fail:
+	dbus_error_free(&err);
+finish:
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
@@ -1760,6 +1897,8 @@ impl_device_add_listener(void *object, struct spa_hook *listener,
 	add_filters(this);
 	get_managed_objects(this);
 
+	this->objects_listed = true;
+
 	if (this->backend_ofono)
 		backend_ofono_add_filters(this->backend_ofono);
 
@@ -1802,6 +1941,8 @@ static int impl_clear(struct spa_handle *handle)
 
 	monitor = (struct spa_bt_monitor *) handle;
 
+	unregister_media_application(monitor);
+
 	spa_list_consume(t, &monitor->transport_list, link)
 		spa_bt_transport_free(t);
 	spa_list_consume(d, &monitor->device_list, link)
@@ -1879,6 +2020,8 @@ impl_init(const struct spa_handle_factory *factory,
 	spa_list_init(&this->device_list);
 	spa_list_init(&this->transport_list);
 
+	register_media_application(this);
+
 	this->backend_hsp_native = backend_hsp_native_new(this, this->conn, support, n_support);
 	this->backend_ofono = backend_ofono_new(this, this->conn, info, support, n_support);
 	this->backend_hsphfpd = backend_hsphfpd_new(this, this->conn, info, support, n_support);