Created attachment 110123 [details] [review]
connect to session bus

One idea is to connect bonobo-activation-server to the session bus, so it will automatically exit when the session exits.

This isn't the only conceivable way to tie it to the session, but it's an easy way.

(Note it's not as nice as it could be because it doesn't let bonobo-activation-server go down its clean up path)

Agreed - we had a nasty regression here IMHO when we moved from 'GOAD' back pre Gnome 2.0 :-)

The basic idea of a per-system service for a per-session thing is basically stupid :-)

Having said that - unfortunately, if you log in twice: sadly you can re-use the same b-a-s (if only by accident).


So - what to do: I like the general approach: but I'd really like to go further.

If we make libbonobo depend on dbus: I'd like to hollow out bonobo-activation, and the activation daemon: and make them normal per-session dbus services.

Perhaps we should still use CORBA for the existing APIs in the 1st pass; but we need to re-hash all the 'DISPLAY' handling and other nastiness [ though this may impact multi-display issues rather unpleasantly, which would suck ].

Hmm ;-)

Perhaps the simplest thing to do is to ensure we propagage DBUS_SESSION_BUS_ADDRESS from each client to the daemon [ there is code to do this already for other env. vars ], and then ensure we connect [ as your patch does ] to each possible session bus, and then cleanup when they are all gone.

How does that sound ?

So my thoughts:
 - If we go with the connect to multiple sessions idea, my approach isn't really sufficient.  It will need to set_exit_on_disconnect (FALSE), watch for the Disconnected signal of each bus connection and only exit when all sessions exit.  May be should do the listen-for-Disconnected-signal thing anyhow though.
 - If we agree the basic idea of having a per-user instead of per-session bonobo-activation-server is misguided, we should fix it.  I don't think having one bonobo-activation-server process running to cover all running sessions for a user is useful, is it?  There's no practical advantage over having multiple bonobo-activation-server processes, one for each session, yea?
 - The multiple sessions to one user case isn't super interesting because things are already pretty broken in other ways if you try to do that.
 - I do think it probably makes sense to keep using CORBA for the existing APIs, just to minimize the potential for breaking things.
 - More generally, and in that same vein, I think it makes sense to keep our changes relatively targeted.
 - Maybe we should rename /tmp/orbit-$USER/bonobo-activation-server-ior to /tmp/orbit-$USER/bonobo-activation-server-$DBUS_SESSION_BUS_ADDRESS-ior (and similarly for the lock file).  

What do you think?

Sounds like you're on top of it to me Ray :-)
My only concern is that the 'screen' mangled into the DISPLAY should get across the wire & be propagated to a new applet say on screen 1.

In fact, forget that - I can't recall the constraints we should be keeping: if you can re-hack it, and test it in some multi-head setups: I'm happy :-) go for it. Pleased to have you involved: [ and yes, keeping the CORBA API for using the daemon itself is prolly a good plan for now ].

Created attachment 110482 [details] [review]
Make bonobo-activation-server per session instead of per user

This patch does 4 things:

1) Exports the get_ior_fname() and get_lock_fname() functions from libbonobo-activation so that activation-server doesn't need to duplicate the logic for figuring out those names

2) Changes the implementation of those two functions to key off of the session guid from DBUS_SESSION_BUS_ADDRESS

3) incorporates the original patch in this bug report that makes activation-server connect to the session bus.  It additionally does the connect to "Disconnected" signal bits we discussed

4) adds a function to delete the ior and lock file on shutdown.  It apparently wasn't doing it before.

Oh one other point I forgot to mention in earlier was, I don't think there should be any semantic shift with regard to multi-head setups between per-session and per-user activation servers, since in both cases we have one bonobo-activation-server running for all screens.

That is to say in a more than once Screen ("zaphod mode") multi-head setup there is normally one dbus-daemon to cover all Screen's, not, e.g., one dbus-daemon for 0.0 and one for 0.1.

I'm guessing it would be good to get this tested as much as possible early on in the 2.23.x cycle? e.g. now?

Ray - looks really good to me; please do commit & thanks for the fix.

commmited:

2008-05-07  Ray Strode  <rstrode@redhat.com>

        * activation-server/activation-server-main.c:
        (cleanup_ior_and_lock_files), (main):
        remove ior and lock files on exit path

2008-05-07  Ray Strode  <rstrode@redhat.com>

        Make bonobo-activation-server exit when dbus session exits

        * configure.in: Require dbus and dbus-glib
        * activation-server/activation-server-main.c:
        (bus_message_handler): Check for "Disconnected" signal and
        quit mainloop upon getting it.
        (main): Get on session bus and filter for messages.

2008-05-07  Ray Strode  <rstrode@redhat.com>

        Key ior and lock filenames of dbus session guid

        * bonobo-activation/bonobo-activation-base-service.c
        (get_session_guid): New function to extract guid from
        $DBUS_SESSION_BUS_ADDRESS
        (_bonobo_activation_lock_fname_get),
        (_bonobo_activation_ior_fname_get): encode result of
        of get_session_guid () into returned filenames

2008-05-07  Ray Strode  <rstrode@redhat.com>

        Consolidate ior and lock name getter logic to one
        place to facilitate changing that logic

        * bonobo-activation/bonobo-activation-private.h:
        * bonobo-activation/bonobo-activation-base-service.c:
        (get_lock_fname), (get_ior_fname),
        (_bonobo_activation_lock_fname_get),
        (_bonobo_activation_ior_fname_get): Rename and export
        get_lock_fname and get_ior_fname.
        (rloc_file_lock), (rloc_file_check), (rloc_file_register),
        (rloc_file_unregister): adapt for renamed functions
        * activation-server/activation-server-main.c
        (dump_ior): get ior name from libbonobo-activation instead
        of hardcoding it to "bonobo-activation-server-ior"