search

bus1 / dbus-broker

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

Noticing new symlinks in service directories #211

Closed owtaylor closed 3 years ago

owtaylor commented 5 years ago

If you symlink a service into a service directory, the directory watch does not trigger, since a new symlink is IN_CREATE - and the watch added on the service directory doesn't look for that.

From reading the code, dbus-daemon seems to have the same behavior - but I think it's still pretty clearly a bug. May be tricky to fix without causing double reloads for common cases.

Ref: https://github.com/flatpak/flatpak/issues/3145

hadess commented 5 years ago

See also #202

dvdhrm commented 5 years ago

Yeah, we can ammend this, but is that really what you want? inotify is async, so there is not guarantee that the bus reloaded the configuration at any time. Moreover, I think even the kernel part of fsnotify is async, so not even prioritizing the inotify-fd in epoll-dispatching would help.

IOW, cant we use ReloadConfig on the bus instead? This is an unprivileged call, if I am not mistaken, and will properly provide a barrier, no?

dvdhrm commented 4 years ago

I am closing this, as there does not seem to be any update on this. Feel free to reopen this, if there is interest again. But please be ready to explain how you avoid races with inotify.

allisonkarlitskaya commented 3 years ago

I'm interested in this being fixed. For its part, flatpak has worked around this issue by creating the symlink with a temporary name and renaming it into place (which is an event that dbus-broker notices).

Your argument about inotify being async the service being (not) available at any point in the future could also apply to the existing events that you watch for, and yet you still watch for them... it seems pretty reasonable to expect to be able to install something and then, at some point after a few seconds, expect it to be working... if you wanted to be really strict, you could wait until the name becomes activatable on the bus...

dvdhrm commented 3 years ago

Heyho Allison!

Your argument about inotify being async the service being (not) available at any point in the future could also apply to the existing events that you watch for, and yet you still watch for them...

dbus-broker just copies behavior from dbus-daemon, as part of the compatibility promise. If you want to know why some events are watched, and others are not (or why automatic configuration reloading is done), I cannot help other than directing you to upstream dbus-daemon developers.

Configuration handling in dbus-broker is part of the compatibility launcher, as such it just mimics whatever dbus-daemon does.

it seems pretty reasonable to expect to be able to install something and then, at some point after a few seconds, expect it to be working... if you wanted to be really strict, you could wait until the name becomes activatable on the bus...

If I was to decide, I would rip out the entire configuration handling and replace it with proper IPC calls. Modifying file-systems is just never atomic (what if you need to install multiple names? What if you need to synchronize configuration updates into multiple different system services?). I know you can make things work with inotify, but I believe it makes it unnecessarily complex for applications to use (i.e., applications now have to be aware that they can be started without all configuration synced up, or their requirements being installed non-atomically, rather than the app-store doing the heavy lifting).

If you feel like this is the way forward, I am in no way stopping you. src/util/dirwatch.c is the place to start, if this is just about adding IN_CREATE. If you want something more elaborate, I would feel better if we do not deviate too much from dbus-daemon.

Thanks! David

dvdhrm commented 3 years ago

I will close this for now, as there seems to be no followup to my previous comment. We follow what dbus-daemon does, but I will gladly join a discussion about extending the capabilities, if you are willing to discuss it.