From 6d33772f9ae6769c557e2267d16b7d31f67db914 Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Thu, 28 Nov 2013 14:58:57 +0100
Subject: [PATCH] logind: restore session-controller after crash

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 | 45 +++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index d3433731e..0e1c40b4e 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -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] = {
-- 
2.30.2