prevent avahi signal racing
ClosedPublic

Authored by sitter on Oct 18 2018, 11:09 AM.

Details

Summary

https://github.com/lathiat/avahi/issues/9

Avahi has upstream signal races which can have any number of side effect
bugs on our end ranging from unreliable discovery, over missing servers
in kio slave listings, to outright deadlocking when waiting for a result
which has already raced passed us (specifically RemoteService is vulnerable
to that).

When we make a request to avahi it will immediately start sending
discovery signals even when we haven't even connected to the signals yet.
This means any number of signals may be lost in the time between our
request and us connecting to the signals.
As this applies to (all?) in-the-wild versions of avahi even if we got an
upstream fix today it'd not help the user until they get the new avahi
integrated.

To prevent us from losing the race all dbus signal listening is now done
via blanket connects.
Ordinarily we'd make a request, dbus would give us an object path, we'd
connect to the object path's signals and then get signals for our request.
Since the signals are racey we'll instead connect to all signals of a given
interface (e.g. org.freedesktop.Avahi.DomainBrowser) completely ignoring
the object path. We'll then "locally" filter all signals which belong to
the specific DomainBrowser object as identified by the signals' dbus object
path. This eliminates all risk of racing. Before we make the request
we setup our blanket connections and the slots won't get called until after
the event loop returns, which won't be until we've set "our" dbus object
path for filtering later in the relevant functions.

The obvious downsides of this are:

  • boilerplatey code for handling this stuff
  • runtime checked signal-slot compatibility
  • much more traffic going into our slots (doesn't actually end up doing any heavy lifting)
Test Plan
  • wrote a crappy test app to use all browser -> still browse (also service resolves)

Diff Detail

Repository
R272 KDNSSD
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Oct 18 2018, 11:09 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 18 2018, 11:09 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Oct 18 2018, 11:09 AM
sitter added inline comments.
src/avahi-remoteservice.cpp
104

FTR: this is obviously leaking and was doing so before as well. I'll fix it after this diff.

Not sure if evil or genius ;)
Looks sane, as sane as it gets, obviously.

src/avahi_listener_p.h
30

*Assists

42

Make const

sitter updated this revision to Diff 43863.Oct 18 2018, 11:18 AM

typo-- && make isOurMsg const

sitter marked 2 inline comments as done.Oct 18 2018, 11:19 AM
broulik accepted this revision.Nov 21 2018, 9:23 AM
This revision is now accepted and ready to land.Nov 21 2018, 9:23 AM
This revision was automatically updated to reflect the committed changes.