D-Bus API Race

[lathiat]

from #gnome-hackers/hadess

The current D-BUS API requires that you create an object, which then immediately starts returning signals.

The C Client appears to subscribe to all signals up front, however the Python API and for example frameworks like GDBUS would have you call 'connect_to_signal' once the object is created and the path is known. Signals might arrive in this time frame.

Investigate further if this actually occurs and what possible fix there might be, other than an obvious Start method. Does d-bus have some way around this, what do others do?

Example in avahi-python/avahi-bookmarks.in:
browser = dbus.Interface(self.bus.get_object(avahi.DBUS_NAME, self.server.ServiceBrowserNew(avahi.IF_UNSPEC, avahi.PROTO_UNSPEC, stype, domain, dbus.UInt32(0))), avahi.DBUS_INTERFACE_SERVICE_BROWSER)
browser.connect_to_signal('ItemNew', self.new_service)
browser.connect_to_signal('ItemRemove', self.remove_service)

[smcv]

If there is no method call that can be used to "catch up", then this can never be reliable with proxy objects (dbus-python's get_object(), GDBus's GDBusProxy, etc.), only with lower-level APIs in which a client adds a non-specific signal match before creating any service browsers.

The conventional way to resolve this in other D-Bus APIs is a GetItems() method that can be used to "catch up".

[lathiat]

Thanks for the comment, I was hoping for info on the conventional method to solve that.

[smcv]

See also: the thread starting at https://mail.gnome.org/archives/gtk-devel-list/2015-July/msg00019.html

[rogerjames99]

I started the thread over at gnome. You can find my workaround here at

https://github.com/rogerjames99/unicornemu/blob/master/avahibrowser.py

[lathiat]

calling GetItems seems to also be racy, as we will never know precisely which was the first signal they received. Some kind of .Start() would be ideal but that would break backwards compatibility.

[smcv]

The idea is that clients follow this sequence:

• connect to signal(s)
• call state-recovery method

which cannot miss any items, but might duplicate them: the worst that can happen is that they receive some signals after calling the state-recovery method, but before it returns.

Duplicate items can be addressed by having the client specifically ignore duplicates, or by having an "initialized" flag as described in https://mail.gnome.org/archives/gtk-devel-list/2015-July/msg00021.html.

[rogerjames99]

I had quick look at the code for the for the use case I have been looking at only. i.e. Creating a Server proxy then calling its ServiceBrowserNew method. In this case after sending back the response to this method the server will always send zero or more ItemNew messages followed by an AllowForNow message. So adding a catch up method would not break backwards compatibility for this case, provided the catch up also broadcast an AllForNow message. A new client implementation can synchronise by not finalising its local cache until it receives an AllForNow message after the catch up call.

I have not looked at the other race mentioned above.

[rogerjames99]

Just a thought. If the catch up broadcast an AllForNow at the start of the catch up sequence that would make client implementation even easier. As old clients that get all the messages should treat two consecutive AllForNow messages as an empty cache.

[smcv]

Suppose my network contains services called alpha, beta and gamma, and Avahi additionally detects delta after it has signalled AllForNow. <- X -> denotes a signal and <= X or X => denotes a method call or reply, Avahi is on the left, and time increases downwards.

The conventional fix for this sort of issue (in pseudocode) is like this:

                                       <= ServiceBrowserNew()
ServiceBrowserNew reply: /foo =>
<- ItemNew(alpha) ->
                                       <= GetItems
<- ItemNew(beta) ->
GetItems reply: alpha, beta =>
<- ItemNew(gamma) ->
<- AllForNow() ->
<- ItemNew(delta) ->

What @rogerjames99 seems to be suggesting something more like this. This would work, but is by no means conventional:

                                       <= ServiceBrowserNew()
ServiceBrowserNew reply: /foo =>
<- ItemNew(alpha) ->
                                       <= RepeatSignals()
<- ItemNew(beta) ->
RepeatSignals reply: (nothing) =>
<- ItemNew(alpha) ->
<- ItemNew(beta) ->
<- ItemNew(gamma) ->
<- AllForNow() ->
<- ItemNew(delta) ->

[smcv]

Another way to think about the conventional solution is this design principle: first write some methods, so that at any time, it's possible to retrieve the state of the object by calling its methods ("state recovery"); then add signals to provide change notification for the results of those methods (ideally, designing them so that the number of times the methods need to be called is minimized).

In the case of Avahi, this probably means that GetItems() should actually return a boolean for whether AllForNow() has been emitted yet, in addition to the actual list of items.

[rogerjames99]

A small correction to @smvc analysis. DBUS is seqential so what I expect to see is this:

                                                             <= ServiceBrowserNew()
ServiceBrowserNew reply: /foo =>
<- ItemNew(alpha) ->*
                                                             <= RepeatSignals()
<- ItemNew(beta) ->*
<- ItemNew(gamma) ->*
<- AllForNow() ->*
RepeatSignals reply: (nothing) =>
<- ItemNew(alpha) ->
<- ItemNew(beta) ->
<- ItemNew(gamma) ->
<- AllForNow() ->
<- ItemNew(delta) ->

or this:

                                                             <= ServiceBrowserNew()
ServiceBrowserNew reply: /foo =>
<- ItemNew(alpha) ->*
<- ItemNew(beta) ->*
<- ItemNew(gamma) ->*
<- AllForNow() ->*
<- ItemNew(delta) ->*
                                                             <= RepeatSignals()
RepeatSignals reply: (nothing) =>
<- ItemNew(alpha) ->
<- ItemNew(beta) ->
<- ItemNew(gamma) ->
<- ItemNew(delta) ->
<- AllForNow() ->

Depending on when the server sees the RepeatSignals(a.k.a CatchUp) request. This signals I have marked with an asterisk can potentially be missed by the client.

On further reflection I think the server should mark the end of the re-broadcast signals with a <- CacheExhausted -> signal not an <- AllForNow ->. So the end result is something like this:

                                                             <= ServiceBrowserNew()
ServiceBrowserNew reply: /foo =>
<- ItemNew(alpha) ->*
<- ItemNew(beta) ->*
<- ItemNew(gamma) ->*
<- AllForNow() ->*
<- ItemNew(delta) ->*
                                                             <= CatchUp()
CatchUp reply: (nothing) =>
<- ItemNew(alpha) ->
<- ItemNew(beta) ->
<- ItemNew(gamma) ->
<- ItemNew(delta) ->
<- CacheExhausted() ->

I will leave it to you guys who are actually familiar with this to decide whether this will break existing clients.

Roger

[rogerjames99]

One last off topic question. Is this now the official repository for avahi? Ubuntu is still citing git://git.0pointer.net/avahi.git as the upstream source.

[smcv]

I still think the version with GetItems() is better than having a RepeatSignals() method (or whatever you want to call the method - it's the meaning that matters).

The approach with GetItems() is certainly more conventional: for instance, the standard D-Bus Properties interface works that way (state-recovery via GetAll() and change-notification via PropertiesChanged).

[rogerjames99]

Either way is fine with me :-)

Maybe we should call the method Gnomic ;-)

Definition of gnomic in English: adjective. Expressed in or of the nature of short, pithy maxims or aphorisms: 'that most gnomic form, the aphorism'

Roger

[soemraws]

I was just wondering if the following I encountered is related to this race condition.

If I create a new ServiceBrowser and immediately register a handler to the signal "ItemNew", I generally do find the services I was expecting. However, if I now unregister the handler and call "Free" on the ServiceBrowser, then create a new ServiceBrowser and re-register a handler to the signal, I never (well after many tries) find the services.

I would expect that when freeing the ServiceBrowser, everything related to it is destroyed. Thus when creating a new ServiceBrowser, we are starting from a "clean slate" again and the original race condition would appear, i.e. I would generally find the service. However, it seems that it's a sure thing that I won't find it.

If it is unrelated, I can open a new issue.

Thanks,
Sumant

[hadess]

It's the same bug, but it's rightfully not behaving as you expect. Avahi, to speed things up, will cache responses to queries. So once you're done looking up a service, it will be quick to look up the second time. That's completely expected.

[soemraws]

Just to be sure I understand correctly, you're saying that due to the caching, in the second case we're sure to find nothing, because the signals are sent out (from the cache) immediately when the browser is instantiated, thus before the handler for the signals is registered?

I've been trying to figure out how to work around this, e.g. by registering the handlers before instantiating the browser. However, it seems that dbus needs the path (of the browser) when registering. Is there any other workaround?

[smcv]

However, it seems that dbus needs the path (of the browser) when registering. Is there any other workaround?

High-level (object-oriented) D-Bus APIs like GDBus' GDBusProxy and dbus-python's dbus.proxies.* need this. There is not going to be a workaround while using high-level D-Bus APIs. You will have to use low-level (message-oriented) APIs.

Quoting myself from https://mail.gnome.org/archives/gtk-devel-list/2015-July/msg00021.html:

Gio.DBusProxy cannot represent this API
successfully, and you will have to use lower-level functions like
GIO.DBusConnection.signal_subscribe to subscribe to the signals in
general, before you call ServiceBrowserNew; and then in the signal
callback, use the sending object path of the signals to associate them
with a particular service browser object.

[soemraws]

If there is not going to be a work-around for high-level D-Bus APIs, at least make the documentation consistent. Quoting from the (online) Avahi documentation:

the D-Bus API: an extensive D-Bus interface for browsing and registering mDNS/DNS-SD services using avahi-daemon. We recommend using this API for software written in any language other than C (e.g. Python).

There are many other languages that only have the object-oriented D-Bus interface. And I would hardly call the D-Bus interface extensive if it cannot work with the high-level (object-oriented) API.

In my opinion, a "Start" method would solve this issue and meet the promise and recommendation in the documentation.

Or, you know, just don't say it's extensive and only works with low-level D-Bus APIs.

[smcv]

I am not an Avahi developer, and even if I was, I couldn't fix the API of already-deployed instances of Avahi.

If you want to contribute a fixed D-Bus API to Avahi so that high-level D-Bus APIs can interact with it, #9 (comment) describes how I would recommend doing so.

Otherwise, this bug ("Avahi's D-Bus API doesn't work with high-level D-Bus APIs") remains open, and the workaround for that bug that can be used outside Avahi is to use low-level D-Bus APIs instead.

[soemraws]

Understood, I didn't mean you personally. I wouldn't mind contributing, if the low level approach you suggested didn't work. But it seems to work very well, so I'll be using that in my code instead. Seems like this will remain open.

Thanks.

[SilaUser]

Are there any intentions to fix this bug in the near future. Maybe by enhancing the DBus API with one of the proposed repeat() or getItems() functions.

[hadess]

If somebody has a test already written, that would help. I have some code written, completely untested, but having a test case would help me avoid having to write one myself.

Python or C using GDbus would be best.

[SilaUser]

Maybe Python Script works as a short test for ServiceBrowser. It demonstrates the race condition in my network.

[ghost]

Is this repository abandoned @lathiat? There are lots of old pull requests without any reaction by the maintainer and the last bigger change happened in july 2017.

[lathiat]

This has now been solved in 0.8 by #175