[pulseaudio-discuss] [PATCH] bluetooth: don't send unsolicted replies to the endpoint Release() call

[Georg Chini]

On 18.01.2018 21:27, Tanu Kaskinen wrote:
> It was reported that PulseAudio causes error messages in syslog from
> dbus-daemon:
>
> Jan 14 04:51:32 gentoo dbus-daemon[2492]: [system] Rejected send message, 2 matched rules; type="error", sender=":1.15" (uid=1000 pid=2864 comm="/usr/bin/pulseaudio --start --log-target=syslog ") interface="(unset)" member="(unset)" error name="org.bluez.MediaEndpoint1.Error.NotImplemented" requested_reply="0" destination=":1.1" (uid=0 pid=2670 comm="/usr/libexec/bluetooth/bluetoothd ")
>
> The default policy on the system bus is to not let any method call
> replies through if they have not been requested, and apparently
> bluetoothd doesn't want replies to the Release() call.
>
> This also changes the reply type from error to normal reply. The "not
> implemented" error didn't make sense to me. We don't do any cleanup in
> the Release() handler, probably because there's nothing to do. If there
> is some cleanup that we should do, then it's a serious bug not to do it.
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=104646
> ---
>   src/modules/bluetooth/bluez5-util.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index cc8d87f9c..2d8337317 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -1641,10 +1641,25 @@ fail:
>   }
>   
>   static DBusMessage *endpoint_release(DBusConnection *conn, DBusMessage *m, void *userdata) {
> -    DBusMessage *r;
> +    DBusMessage *r = NULL;
>   
> -    pa_assert_se(r = dbus_message_new_error(m, BLUEZ_MEDIA_ENDPOINT_INTERFACE ".Error.NotImplemented",
> -                                            "Method not implemented"));
> +    /* From doc/media-api.txt in bluez:
> +     *
> +     *    This method gets called when the service daemon
> +     *    unregisters the endpoint. An endpoint can use it to do
> +     *    cleanup tasks. There is no need to unregister the
> +     *    endpoint, because when this method gets called it has
> +     *    already been unregistered.
> +     *
> +     * We don't have any cleanup to do. */
> +
> +    /* Reply only if requested. Generally bluetoothd doesn't request a reply
> +     * to the Release() call. Sending replies when not requested on the system
> +     * bus tends to cause errors in syslog from dbus-daemon, because it
> +     * doesn't let unexpected replies through, so it's important to have this
> +     * check here. */
> +    if (!dbus_message_get_no_reply(m))
> +        pa_assert_se(r = dbus_message_new_method_return(m));
>   
>       return r;
>   }

LGTM