Bug 104646

Crap I misspelled DBUS as DUS in the title - oops.  Can't edit that now myself, but someone can change it if they feel the urge.

I agree that we shouldn't respond with an error, but I believe that's not enough to make the log spam go away. The D-Bus daemon should treat both success and failure replies in the same way. I think the relevant thing is that bluetoothd didn't request a reply to the Release() call, and the D-Bus daemon is not allowing any replies in such case.

Are you able to test a patch if I write one?

I'm not really very familiar with DBus, so I hadn't considered that it might just be because the error was unsolicited (I thought perhaps an exception was never a requested reply), but yes that sounds reasonable.

I can test a patch quite easily - a fresh reboot reliably produces the message for me.

Created attachment 136793 [details] [review]
0001-bluetooth-don-t-send-unsolicted-replies-to-the-endpo.patch

Thanks for the offer to test! Bluetooth works so badly and inconsistently on my machine that I dislike the idea of testing anything bluetooth related myself.

A patch is attached.

Yes, that seems to work.

However, I think it's not the best design to check dbus_message_get_no_reply in endpoint_release.  That means there will be dozens of other places that construct dbus responses that remain unprotected.

I think it preferable to find a place higher up that call tree that is common to all dbus message handlers (or at least all bluez ones) that can check the flag and destroy any unsolicited message that has been constructed and returned by the individual handlers.

That would fix this, but also every other possible instance.

Thanks for testing, I submitted the patch to the mailing list now.

It's a valid point that it's very easy to forget to check the no_reply flag if the checking isn't centralized. That said, it's also very rare that a method call doesn't expect a reply. I'm not motivated enough to work on this.

If someone else wants to work on it, here are my thoughts about the possible implementation:

Ideally libdbus would implement the checking in dbus_connection_send(). Maybe it can't be changed to do that unconditionally, but applications could set an option in the DBusConnection object if they want to enable the filtering.

If the libdbus maintainers reject the feature, then we can do the filtering in PulseAudio, although I don't see any pretty solutions. I can think of two not-so-pretty solutions:

1) Ban the direct use of dbus_connection_send(), and use a wrapper function instead. The wrapper function would take a boolean argument indicating whether or not to actually send the message. That would force every caller to think about whether or not the message should be sent. It would be nice if the send wrapper could get the no_reply flag directly from the message that is being sent, so that the explicit boolean argument wouldn't be needed, but libdbus doesn't seem to provide a function for getting the "in-reply-to" message from the reply message. That feature might be feasible to get added to libdbus, though.

2) Ban the direct use of dbus_message_new_method_return(), dbus_message_new_error() and dbus_message_new_error_printf(), and use wrapper functions instead. The wrapper functions would fail if the original method call didn't request a reply.

The patch was applied, so I'm closing this bug.