bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
675 stars 78 forks source link

libratbag test suite fails with dbus-broker #206

Closed whot closed 5 years ago

whot commented 5 years ago

Still trying to figure out what exactly is happening here, but: the libratbag test suite and debugging commands fail on Fedora 30 with dbus-broker but works with dbus-daemon. This appears to be a race with services dropping dbus policy files.

dbus-broker-21-3.fc30.x86_64
dbus-daemon-1.12.16-1.fc30.x86_64

Reproducible with the libratbag repo

$ git clone https://github.com/libratbag/libratbag
$ cd libratbag
$ meson builddir
$ sudo ninja -C builddir test

And the debugging command ratbagctl.devel

$ sudo ./builddir/ratbagctl.devel list

The latter was a bit easier to debug, it works like this:

With dbus-broker, I get "permission denied" when ratbagd calls sd_bus_request_name(). Eventually I found out that adding a time.sleep(0.2) after installing the .conf file fixes the issue [1] Not always though and it is a heisenbug because it seems to work better with 0.2s than 2s which makes no sense. Once it worked once, it tends to keep working until the git sha changes, but that too is unpredictable.

As for the ninja test command - no idea why that fails, even with timeouts. Printf debugging shows that the name owner is correct and works, but at some point stops working for some tests. dbus-monitor does show a bunch of data exchange before it all falls apart. I'm assuming that it's the same underlying issue.

[edit: the ninja test failure was caused by a different bug]

[1] see tools/toolbox.py:start_ratbagd, the shutil.copy command

teg commented 5 years ago

If you do a manual ConfigReload dbus call instead of sleep, does that solve the issue?

whot commented 5 years ago

silly question but - how do I do that?

Keruspe commented 5 years ago

You can systemctl reload dbus-broker.service for example, whic (according to systemcl cat) is running /usr/bin/busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus ReloadConfig

teg commented 5 years ago

Yes, what @Keruspe suggested is the easiest. Could also do the same busctl call manually.

To give a bit more context: the inotify support is purely there for backwards compatibility. Both on dbus-daemon and on dbus-broker it is racy. We do not check the filesystem for changes on every transaction, rather we watch them with inotify, and reload as soon as a change is detected (which is in no way ordered against message transactions).

The ReloadConfig call (unlike SIGHUP and inotify) is blocking, and will only return after the config reload has been completed. As such, you can use it for synchronization.

The overall logic is the same in dbus-broker and dbus-daemon, so if you install new policy on a running system, you should always run ReloadConfig to ensure it has been fully loaded, after you install all the policy files and before you start relying on them.

The reason you may experience the race more on dbus-broker than on dbus-daemon is that we do the config loading out-of-process, and only pause the bus to replace the config once it has been fully loaded, whereas dbus-daemon blocks the bus the moment an inotify event has been received.

whot commented 5 years ago

Thanks, this does work and I've submitted a PR now to fix this.

FWIW, the ninja test issue seems to be unrelated and/or a combination of this bug and another one (memory corruption) which I've fixed now too. So all is good and we can close this bug. Thanks for the help!