[patch] Auto session bus launch from libdbus, take 2

[Havoc Pennington]

Hi,

Thiago Macieira wrote:
> Since this is an internal token, I even considered changing it 
> to "the_magic_method_that_you_should_never_see". :-)

I think it's fairly clean to make this method public - for example, if 
you wanted to have the bus only listen on local sockets and autostart 
for remote apps, but we fixed ssh to forward the env variable, then you 
might set DBUS_SESSION_BUS_ADDRESS=unix:whatever,autolaunch:

We'd simply document that the default address if the env variable is 
unset is "autolaunch:"

> Indeed. _dbus_transport_open should pass control to the platform-specific 
> version if the none of the known protocols match. That will probably be 
> just TCP or not even that. AFAIR from Winsocks, it's so different and so 
> full of pitfalls that it should probably be in a separate file anyways. 
> That would mean almost all of DBusTransport would be platform-specific.
> 
> (cf the email someone has just posted on the subject)

The winsock/unix-sockets differences are hypothetically buried behind 
dbus-sysdeps.h API, and then most of dbus-transport-unix.c fits nicely 
into dbus-transport-socket.c - I cleaned it all up now so you can just 
hook into what's there in dbus-transport-unix.c. The actual unix API 
usage (libc, unistd.h) should go in dbus-sysdeps-unix.[hc] then be used 
in transport-unix.c

>>> +{
>>> +  static char *cached_address = 0;
>> I'm not sure this cache is needed, since the session bus is stored
>> globally in dbus-bus.c, once the connection is open we won't try to open
>> the transport again. 
> 
> Not really. dbus-bus.c still knows only "session:" and there are private 
> session busses. But since this cache is a potential memory leak because 
> we don't have an atomic test-and-set operation available, maybe it's 
> better not to have it at all. Once the session file is read, the fork 
> penalty should go away.

We could fix the memory leak with a global lock, and also add a shutdown 
hook to free the string on shutdown, it just seems like more trouble 
than it's worth.

>> I think something more descriptive than --x11-integration would be good.
>> Maybe --exit-with-x-session and/or --use-existing-x-property ?
> 
> --exit-wth-session already implies "exit with X session".
> 
> Maybe
> --enable-x11-integration?
> --enable-autolaunch?
> --autolaunch?
> 
> If we don't say anything about X11 there, the code on libdbus would 
> require no modifications to support MacOS X mechanisms.

--autolaunch sounds good, especially if the address is autolaunch:

The man page should say "don't use this, libdbus is the only one that 
should use it" probably.

> No, this is correct. The session file is per user, per host and per 
> display. If I am on machine A.net and my DISPLAY is B.net:0.0, then the 
> file should be called A.net_B.net_0.
> 
> A.net__0 means display :0 running on A.net (local display)
> B.net__0 means display :0 running on B.net (local display)

Weird... but ok, I don't have a better idea if we punt on guid stuff for 
now.

> BTW: dbus-daemon can sometimes exit and dbus-launch doesn't realise that. 
> I guess it's a bug.

Yes, if you can figure out how to reproduce please file a bug. Maybe try 
"kill -11" and such to try killing it different ways.

>>> +      XGetWindowProperty (xdisplay, owner, pid_atom, 0, sizeof pid,
>>> False, +                          XA_CARDINAL, &type, &format, &items,
>>> &after, +                          (unsigned char **) &data);
>>> +      if (type != None && after == 0 && data != NULL && format == 32)
>>> +        *pid = *(pid_t*)data;
>> I think XGetWindowProperty has surprising behavior on 64-bit but I don't
>> remember offhand what it is. It's possible that "data" there is always a
>> long even though it's format "32" - I'd have to look at some other code
>> and see.
> 
> If sizeof(pid_t) == 8, then nitems will be 2. However, I hadn't thought of 
> unaligned access.

Looking at metacity, indeed I think data is always longs. So the bizarre 
thing is that format==32 but each integer is in fact 64 bits on 64-bit 
platforms.

So you need a thing like:
  *pid = (pid_t) *(long*) data;

Havoc