Bug 35887

For the system bus, sure. I think it should be a /run/dbus/ directory so it can contain both the socket and the pid file (for init systems that still need those), and any other clutter we need later (Colin's hypothetical secondary debugging/eavesdropping socket, maybe).

My only worry about this is that /var/run/dbus/system_bus_socket is probably quite popularly hard-coded, as the way you get to the "real" system bus even when installed in a prefix or whatever. Do in-filesystem sockets work with bind-mounts, symlinks, etc.?

---------------------

For the session bus, we can't use /run/$USER/dbus if we still support parallel sessions for one user. I think the unix:tmpdir transport mode is fine, tbh.

We could consider adding a "xdgruntime" transport which declines to listen and fails over to the next transport if XDG_RUNTIME_DIR is unset, which would make the default fallback order be something like:

    xdgruntime:
    unix:tmpdir=/tmp
    autolaunch:

I think that'd do more or less the right thing?

(In reply to comment #1)
> For the system bus, sure. I think it should be a /run/dbus/ directory so it can
> contain both the socket and the pid file (for init systems that still need
> those), and any other clutter we need later (Colin's hypothetical secondary
> debugging/eavesdropping socket, maybe).
> 
> My only worry about this is that /var/run/dbus/system_bus_socket is probably
> quite popularly hard-coded, as the way you get to the "real" system bus even
> when installed in a prefix or whatever.

In fact I just recently did that in jhbuild...
https://bugzilla.gnome.org/show_bug.cgi?id=645187

It's like you're psychic =)

> Do in-filesystem sockets work with
> bind-mounts, symlinks, etc.?

Yeah.

> For the session bus, we can't use /run/$USER/dbus if we still support parallel
> sessions for one user. I think the unix:tmpdir transport mode is fine, tbh.

I think the conclusion on this was that it's an OS policy decision.  
 
> We could consider adding a "xdgruntime" transport 

I'd rather add a configure flag like --enable-single-session which does whatever we want it to do (that may be /run/$USER/dbus).  However - "single session" needs some more work than that.  Basically consider the local GDM + remote ssh case - if the session bus no longer exits after you log out from GDM, then we're back to processes hanging around.  If it does exit, then it breaks ssh.  So we need logic to close connections only from that particular "head" via ConsoleKit.

So this bug got sidetracked because what Jon is concerned about isn't the user agent socket, it's the autolaunch: bits written into ~/.dbus/ by dbus-launch-x11.

We should move that into XDG_RUNTIME_DIR for sure.

Created attachment 60706 [details] [review]
dbus-launch: Move ~/.dbus autolaunch stuff to /run/user

We shouldn't be polluting the user's home directory when we have a
per-machine per-user directory available in XDG_RUNTIME_DIR.

Comment on attachment 60706 [details] [review]
dbus-launch: Move ~/.dbus autolaunch stuff to /run/user

Review of attachment 60706 [details] [review]:
-----------------------------------------------------------------

What reads these files? What writes these files? Are you breaking interoperability by doing this?

If the answer is "only dbus-launch-x11 reads or writes these files" then you're still breaking interop between dbus-launch 1.4 and 1.6. Scenario: I started my session (writing files) under dbus-launch 1.4 then upgraded; now dbus-launch only reads the new location.

If these files are needed by X11 autolaunch (I was under the impression the X-root-window-properties dance was meant to make dealing with files unnecessary?), one fix would be to write the new location, but read both locations. If you start a session under dbus-launch 1.6 and then downgrade to 1.4, I think it's reasonable to expect you to restart your session before things will fully work.

If these files are needed now but don't have to be, because we could use X11 as a synchronization point instead, I think I'd prefer to do that (unless there are Wayland-related reasons not to); dbus-launch-x11 will still have to read the files for interop with sessions started under 1.4, but it can stop writing altogether, and use X11 as "the new location".

If these files are not needed, let's just not write them, and use X11 as the synchronization point.

To be honest I'm pretty confused by dbus-launch-x11, and I think it would benefit greatly from someone writing down exactly how it works. I'll see whether I can reverse-engineer what the spec should have said about it.

While you're in the vicinity of dbus-launch I would really, really appreciate some review on Bug #39196 and Bug #39197.

::: tools/dbus-launch-x11.c
@@ +95,5 @@
> +{
> +  const char *arg;
> +  const char *next;
> +  char *p;
> +  char buf[PATH_MAX+1];

There are three categories of system regarding PATH_MAX:

* those that genuinely do have a fixed limit (Windows is the
  only concrete example I know of)

* those where the kernel has no limit, so as required by POSIX,
  the macro is undefined and naive code like this won't compile
  (Hurd)

* those where the kernel has no limit, but the libc
  implementation disregards POSIX and picks an arbitrary number
  for PATH_MAX as a convenient lie so naive code will compile
  (Linux, Mac OS X, probably *BSD)

For instance see the first Google hit for "path_max":

http://insanecoding.blogspot.co.uk/2007/11/pathmax-simply-isnt.html

If you intend the answer to be "it's OK because every string is limited-length and there are a limited number of them", please verify that on entry.

Or, use DBusString. I'm pretty sure we have sysdeps functions for this.

@@ +106,5 @@
> +  arg = first;
> +  do
> +    {
> +      next = va_arg (args, const char *);
> +      strcpy (p, arg);

Overflows and corrupts the stack if arg is long.

@@ +139,5 @@
> +
> +  while ((arg = va_arg (args, const char *)) != NULL)
> +    {
> +      char *end = result + strlen (result);
> +      result = realloc (result, strlen (result) + strlen (arg) + 1);

Segfaults if realloc returns NULL, which (by policy) D-Bus considers to be a thing that can happen. Please at least call a non-returning oom() function that whines and exits nonzero.

I believe tools/ are allowed to use the static libdbus-internal.la, which gives them access to _dbus_everything. If so, couldn't you just use DBusString?

@@ +167,5 @@
> +        autolaunch_data_dir = wish_i_had_g_build_filename (datadir, "dbus-autolaunch", NULL);
> +      else
> +        {
> +          home = get_homedir ();
> +          autolaunch_data_dir = wish_i_had_g_build_filename (home, ".dbus", "session-bus", NULL);

I think this deserves a comment: "Fall back to the old paths; if you don't want your home directory cluttered with this stuff, set XDG_RUNTIME_DIR", or something.

The files are created as a helper for D-Bus autolaunch implementations that wish to avoid connecting to X11 (e.g., the dbus-java implementation). So if you move them without symlinking ~/.dbus to the new location, those implementations will break.

So do not move them.

Simon, here's the info you may need for documenting the launch. The way the X11 autolaunch works is like this:

 - dbus-launch keeps a window open, which contains two properties:
   _DBUS_SESSION_BUS_ADDRESS
   _DBUS_SESSION_BUS_PID
 - the window can be found by querying X for the window holding the selection:
   _DBUS_SESSION_BUS_SELECTION_${USER}_${MACHINEID}

The reason for doing that instead of saving in the root window is that, if the dbus-launch process exits, it auto-cleans up. The window it had open will be closed and the selections connected to that window will be cleared. There's never any stale data.

The files may contain stale data. They are provided as an optimisation for a process that wishes to check if there's a session bus started already before forking and launching dbus-launch. It needs to handle a failed connection to the address read from them.

The protocol for creating such is that dbus-launch will:

 1. check if the properties are there
 2. if they are there, return them and exit
 3. start a daemon, get its address and PID
 4. create the window, create the properties and save them on the window.
 5. XGrabServer, get the selection again and if it is still unset, set it to the window; XUngrabServer
 6. if the selection was empty, also save the files in ~/.dbus
 7. return the contents of the window the selection points to

(In reply to comment #6)
> The files are created as a helper for D-Bus autolaunch implementations that
> wish to avoid connecting to X11 (e.g., the dbus-java implementation).

The reimplementations that I'm aware of are ndesk-dbus (CLI/C#/.net), dbus-sharp (a fork of ndesk-dbus), dbus-java, GDBus and ruby-dbus.

These bindings use libdbus, so are not affected: Net::DBus (Perl), dbus-python, QtDBus, dbus-glib, dbus-c++, edbus (Enlightenment), libnih (as used in Upstart), dbus-ada, eggdbus, clisp-module-dbus, dbus-ocaml.

(I'm using the approximation that if it's not in Debian, it doesn't exist.)

I wonder whether we can we make those implementations run dbus-launch --autolaunch, like GDBus does, if they notice that they're "probably under X" and they don't already? That does require dbus-launch --autolaunch to be a stable interface - but it basically is already.

> So if you
> move them without symlinking ~/.dbus to the new location, those
> implementations will break.
> 
> So do not move them.

If by "break" you mean "not be optimally fast", I can live with that.

If by "break" you mean cease to work, then I think this probably has to be WONTFIX, and it's up to the people who want this change to happen (Jon and Colin) to research how much will break and how seriously, and if the answer is "only incredibly niche situations", argue in its favour.

> Simon, here's the info you may need for documenting the launch.

Thanks!

(In reply to comment #6)
> The files are created as a helper for D-Bus autolaunch implementations that
> wish to avoid connecting to X11 (e.g., the dbus-java implementation). So if you
> move them without symlinking ~/.dbus to the new location, those implementations
> will break.

Ugh...well, the details of autolaunch: aren't part of the official spec, more an implementation detail of libdbus.  

> The reimplementations that I'm aware of are ndesk-dbus (CLI/C#/.net),
> dbus-sharp (a fork of ndesk-dbus), dbus-java, GDBus and ruby-dbus.

Note however that e.g. GDBus just forks off dbus-launch --autolaunch, so it will transparently pick up the fix here.  So we're interested in the set of dbus reimplementations that:

1) Support autolaunch: at all
2) Reimplement the autolaunch: logic from dbus-launch

Simon since you have the list, do you have their sources handy?

> I wonder whether we can we make those implementations run dbus-launch
> --autolaunch, like GDBus does, if they notice that they're "probably under X"
> and they don't already? That does require dbus-launch --autolaunch to be a
> stable interface - but it basically is already.

OK, yes, this =)

> If by "break" you mean cease to work, then I think this probably has to be
> WONTFIX, and it's up to the people who want this change to happen (Jon and
> Colin) to research how much will break and how seriously, and if the answer is
> "only incredibly niche situations", argue in its favour.

I think most distributions/OSes now properly set up the dbus session bus, so the original use case of "launch KDE app in WindowMaker" is pretty dead.

However, autolaunch: remains for the "su -" and "ssh login" cases.  Even though launching a dbus-daemon under ssh is problematic for many reasons.

(In reply to comment #7)
> > So if you
> > move them without symlinking ~/.dbus to the new location, those
> > implementations will break.
> > 
> > So do not move them.
> 
> If by "break" you mean "not be optimally fast", I can live with that.
> 
> If by "break" you mean cease to work, then I think this probably has to be
> WONTFIX, and it's up to the people who want this change to happen (Jon and
> Colin) to research how much will break and how seriously, and if the answer is
> "only incredibly niche situations", argue in its favour.

Those implementations could, so far, assume that if the file wasn't there, then the daemon wasn't running. If that was the case, they'd launch it with --autolaunch, which would start it and return the address.

Therefore, I guess it falls under the "not be optimally fast" case, which means we could go ahead anyway.

Except if the code is broken and tries to read off the files even after launching dbus-launch. That is, if they ignore the output of the launcher process, which is the address that the application should connect to.

Is this stuck, or are we still waiting for a final consensus here ?

(In reply to comment #10)
> Is this stuck, or are we still waiting for a final consensus here ?

One thing that sucks about hacking on the dbus tools (dbus-launch, dbus-monitor) is that you can only use libc functions, since the internal "poor man's GLib" bits are private to libdbus.

We could work around that in two ways:

1) Separate out a *private* libpoor-mans-glib-under-AFL.so that's linked to by both libdbus.so and dbus-launch/dbus-monitor.
2) Make a static convenience library of some of the core stuff, link it into both

(In reply to comment #11)
> 2) Make a static convenience library of some of the core stuff, link it into
> both

That already exists, it's libdbus-internal.la.

Each binary may link (directly or indirectly) either libdbus-1.la (usually shared) or libdbus-internal.la (always static, contains additional objects compiled from *-util.c), but never both.

*ping

(In reply to comment #10)
> Is this stuck, or are we still waiting for a final consensus here ?

For the question of "can we move these without breaking compatibility?" the onus is on people in favour of this change to research what, if anything, it will break, and how that breakage can be avoided.

For the question of "if we do move stuff, is this patch OK?", the answer is "not really"; see Comment #5 from the first ":::" onwards.

*** Bug 64704 has been marked as a duplicate of this bug. ***

I wish I knew C better so that I could correct the patch to make it acceptable.

I do have a suggestion for the "breaking compatibility" argument.  Until the patch is fixed and proper testing is done, can the DBUS_DIR variable be exposed in a way that it can be overridden by an environment variable?  Here is what I'm thinking.

export DBUS_DIR=/home/carl/.config/dbus

This approach would serve two purposes.  It would assist in testing for compatibility issues of moving that configuration, and it would (temporarily) address the "polluting the user's home directory" issue.

(In reply to Colin Walters from comment #2)
> I'd rather add a configure flag like --enable-single-session which does
> whatever we want it to do (that may be /run/$USER/dbus).

We now have this for systems with systemd and systemd-logind. It's selected via "./configure --enable-user-session" and listens on "$XDG_RUNTIME_DIR/bus".

This does not use dbus-launch, therefore it does not write autolaunch stuff to ~/.dbus, solving the original scope of this bug report on those systems.

The necessary features to make this work on non-systemd systems, if anyone wants to implement that, would be:

* something similar to https://launchpad.net/pam-xdg-support to create an
  XDG_RUNTIME_DIR;
* something started at login (another PAM module? an Xsession.d hook?)
  arranging for a dbus-daemon --session to be available at
  $XDG_RUNTIME_DIR/bus, or listening on the socket and handing it over to
  a dbus-daemon when there is a connection

(On systemd, the combination of libpam-systemd, systemd-logind and systemd --user provide both of those for us.)

(In reply to Carl George from comment #16)
> I do have a suggestion for the "breaking compatibility" argument.  Until the
> patch is fixed and proper testing is done, can the DBUS_DIR variable be
> exposed in a way that it can be overridden by an environment variable?

Not really. The same modules that might be relying on ~/.dbus would not be aware of the DBUS_DIR variable either, so compatibility would still be broken.

Either ~/.dbus is needed, in which case we have to keep it; or it isn't, in which case someone who cares about this change (and the dbus-launch code path) needs to do the research to demonstrate that. In a sense, making its location per-system-configurable is the worst of both worlds: it breaks anything that was relying on it, but we still have to write out that information to some other location.

If you don't like your home directory being polluted by ~/.dbus, I would recommend not using dbus-launch. For instance, dbus-launch can be avoided in Debian with the dbus-user-session package installed, Fedora 24+, or recent Arch Linux.

(In reply to Simon McVittie from comment #17)
> If you don't like your home directory being polluted by ~/.dbus, I would
> recommend not using dbus-launch. For instance, dbus-launch can be avoided in
> Debian with the dbus-user-session package installed, Fedora 24+, or recent
> Arch Linux.

I took a look at that package and what is does is pretty much create a dbus user session that is managed by the systemd user instance.
In that case, if the ~/.dbus directory is not created, won't modules that depend on it fail? So, anything that relies on it will be broken if you use that package?

(In reply to temp from comment #18)
> I took a look at that package and what is does is pretty much create a dbus
> user session that is managed by the systemd user instance.
> In that case, if the ~/.dbus directory is not created, won't modules that
> depend on it fail? So, anything that relies on it will be broken if you use
> that package?

Yes, to some extent the "user session" setup breaks traditional expectations. That's why it isn't enabled by default, and is something that system integrators or sysadmins have to opt-in to. Some system integrators (Arch, Fedora) opt-in unconditionally, others (Debian) leave it up to the local sysadmin.

The expectation is that system integrators use system-specific mechanisms (e.g. Debian's /etc/X11/Xsession.d) to ensure that DBUS_SESSION_BUS_ADDRESS is set, which mitigates this potential incompatibility by avoiding the need for non-libdbus implementations to look in ~/.dbus or run dbus-launch at all. For example, <https://anonscm.debian.org/cgit/pkg-utopia/dbus.git/tree/debian/20dbus_xdg-runtime> is the implementation used in Debian's dbus-user-session.

(In reply to Simon McVittie from comment #17)
> Either ~/.dbus is needed, in which case we have to keep it; or it isn't, in
> which case someone who cares about this change (and the dbus-launch code
> path) needs to do the research to demonstrate that.

I'd be happy to review a change based on someone carrying out and summarizing this research, but I don't have enough time available for dbus work to do it myself.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/46.