chiark / gitweb /
logind: restore session-controller after crash
authorDavid Herrmann <dh.herrmann@gmail.com>
Thu, 28 Nov 2013 13:58:57 +0000 (14:58 +0100)
committerDavid Herrmann <dh.herrmann@gmail.com>
Thu, 28 Nov 2013 14:16:49 +0000 (15:16 +0100)
We now save the unique bus-name of a session-controller as CONTROLLER=%s
in the session files. This allows us to restore the controller after a
crash or restart.

Note that we test whether the name is still valid (dbus guarantees that
the name is unique as long as the machine is up and running). If it is,
we know that the controller still exists and can safely restore it. Our
dbus-name-tracking guarantees that we're notified once it exits.

Also note that session-devices are *not* restored. We have no way to know
which devices where used before the crash. We could store all these on
disk, too, or mark them via udev. However, this seems to be rather
cumbersome. Instead, we expect controllers to listen for NewSession
signals for their own session. This is sent on session_load() and they can
then re-request all devices.

The only race I could find is if logind crashes, then the session
controller tries calling ReleaseControl() (which will fail as logind is
down) but keeps the bus-connection valid for other independent requests.
If logind is restarted, it will restore the old controller and thus block
the session.
However, this seems unlikely for several reasons:
 - The ReleaseControl() call must occur exactly in the timespan where
   logind is dead.
 - A process which calls ReleaseControl() usually closes the
   bus-connection afterwards. Especially if ReleaseControl() fails, the
   process should notice that something is wrong and close the bus.
 - A process calling ReleaseControl() usually exits afterwards. There may
   be any cleanup pending, but other than that, usual compositors exit.
 - If a session-controller calls ReleaseControl(), a session is usually
   considered closing. There is no known use-case where we hand-over
   session-control in a single session. So we don't care whether the
   controller is locked afterwards.

So this seems negligible.

src/login/logind-session.c

index d3433731e56c48a57e9ade7e72be24351f668674..0e1c40b4eb8ab87418dfcfbe4a282f15431d84a9 100644 (file)
@@ -239,6 +239,9 @@ int session_save(Session *s) {
                         (unsigned long long) s->timestamp.realtime,
                         (unsigned long long) s->timestamp.monotonic);
 
+        if (s->controller)
+                fprintf(f, "CONTROLLER=%s\n", s->controller);
+
         fflush(f);
 
         if (ferror(f) || rename(temp_path, s->state_file) < 0) {
@@ -263,7 +266,8 @@ int session_load(Session *s) {
                 *class = NULL,
                 *uid = NULL,
                 *realtime = NULL,
-                *monotonic = NULL;
+                *monotonic = NULL,
+                *controller = NULL;
 
         int k, r;
 
@@ -287,6 +291,7 @@ int session_load(Session *s) {
                            "UID",            &uid,
                            "REALTIME",       &realtime,
                            "MONOTONIC",      &monotonic,
+                           "CONTROLLER",     &controller,
                            NULL);
 
         if (r < 0) {
@@ -387,6 +392,11 @@ int session_load(Session *s) {
                         s->timestamp.monotonic = l;
         }
 
+        if (controller) {
+                if (bus_name_has_owner(s->manager->bus, controller, NULL) > 0)
+                        session_set_controller(s, controller, false);
+        }
+
         return r;
 }
 
@@ -967,6 +977,25 @@ bool session_is_controller(Session *s, const char *sender) {
         return streq_ptr(s->controller, sender);
 }
 
+static void session_swap_controller(Session *s, char *name) {
+        SessionDevice *sd;
+
+        if (s->controller) {
+                manager_drop_busname(s->manager, s->controller);
+                free(s->controller);
+                s->controller = NULL;
+
+                /* Drop all devices as they're now unused. Do that after the
+                 * controller is released to avoid sending out useles
+                 * dbus signals. */
+                while ((sd = hashmap_first(s->devices)))
+                        session_device_free(sd);
+        }
+
+        s->controller = name;
+        session_save(s);
+}
+
 int session_set_controller(Session *s, const char *sender, bool force) {
         char *t;
         int r;
@@ -989,28 +1018,18 @@ int session_set_controller(Session *s, const char *sender, bool force) {
                 return r;
         }
 
-        session_drop_controller(s);
+        session_swap_controller(s, t);
 
-        s->controller = t;
         return 0;
 }
 
 void session_drop_controller(Session *s) {
-        SessionDevice *sd;
-
         assert(s);
 
         if (!s->controller)
                 return;
 
-        manager_drop_busname(s->manager, s->controller);
-        free(s->controller);
-        s->controller = NULL;
-
-        /* Drop all devices as they're now unused. Do that after the controller
-         * is released to avoid sending out useles dbus signals. */
-        while ((sd = hashmap_first(s->devices)))
-                session_device_free(sd);
+        session_swap_controller(s, NULL);
 }
 
 static const char* const session_state_table[_SESSION_STATE_MAX] = {