logind: track user service managers as 'manager' session class

Previously, all user code was part of a session except for the code run
as part of user@.service, which wasn't. This tries to make this more
uniform: we'll track the user@.service runtime also as a session, but of
the special type "manager".

This means we have a really good overview finally of all user code that
is running and can make decisions on what to start when and how long to
keep it around. The pam_systemd client side will now be reasonably
uniform: it just calls the CreateSession() bus call with the right
class, and we'll return any data it needs. This means the weird
"side-channel" we previously used to initialize XDG_RUNTIME_DIR for the
user@.service goes away (see next commit).

This conditionalizes various behaviours now cleanly depending on the
session class:

1. SESSION_CLASS_WANTS_SCOPE() will be true for all classes except for
   the manager class. It declares whther the client shall be migrated
   into their own scope, which we generally want for sessions but not
   for the manager, since it already has its own service unit.

2. SESSION_CLASS_WANTS_SERVICE_MANAGER() will be true for all classes
   except for the manager class. It declares whether we shall start the
   service manager if a session of this class is around. Of course, this
   is off for the service manager, since this would always pin itself.

3. SESSION_CLASS_PIN_USER() will be true for all classes except for the
   manager class. It declares whether the we shall keep the User
   structure around for a user as long as the session is around.

Now you might wonder why have these as three functions, even though they
mostly give the same answers?

That's because this all is preparation to add further session classes
later that will return different answers for the three calls. (For
example, a later patch adds "background-light" which will return true
for SESSION_CLASS_WANTS_SCOPE() and SESSION_CLASS_PIN_USER(), but false
for SESSION_CLASS_WANTS_SERVICE_MANAGER(). i.e. it will get a scope, and
pin user tracking, but not start a service manager.

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4f8525b46a16788ada0ae925dc1ca781ca6e1f49..31efb0d113dbb020d8e5a22ad0bccc09fdaf5639 100644 (file)
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -865,25 +865,19 @@ static int create_session(
                         c = SESSION_USER;
         }
 
-        /* Check if we are already in a logind session. Or if we are in user@.service
-         * which is a special PAM session that avoids creating a logind session. */
-        r = manager_get_user_by_pid(m, leader.pid, NULL);
+        /* Check if we are already in a logind session, and if so refuse. */
+        r = manager_get_session_by_pidref(m, &leader, /* ret_session= */ NULL);
         if (r < 0)
                 return r;
         if (r > 0)
                 return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY,
                                          "Already running in a session or user slice");
 
-        /*
-         * Old gdm and lightdm start the user-session on the same VT as
-         * the greeter session. But they destroy the greeter session
-         * after the user-session and want the user-session to take
-         * over the VT. We need to support this for
-         * backwards-compatibility, so make sure we allow new sessions
-         * on a VT that a greeter is running on. Furthermore, to allow
-         * re-logins, we have to allow a greeter to take over a used VT for
-         * the exact same reasons.
-         */
+        /* Old gdm and lightdm start the user-session on the same VT as the greeter session. But they destroy
+         * the greeter session after the user-session and want the user-session to take over the VT. We need
+         * to support this for backwards-compatibility, so make sure we allow new sessions on a VT that a
+         * greeter is running on. Furthermore, to allow re-logins, we have to allow a greeter to take over a
+         * used VT for the exact same reasons. */
         if (c != SESSION_GREETER &&
             vtnr > 0 &&
             vtnr < MALLOC_ELEMENTSOF(m->seat0->positions) &&
@@ -1017,8 +1011,14 @@ static int create_session(
 
         session->create_message = sd_bus_message_ref(message);
 
-        /* Now, let's wait until the slice unit and stuff got created. We send the reply back from
-         * session_send_create_reply(). */
+        /* Now call into session_send_create_reply(), which will reply to this method call for us. Or it
+         * won't – in case we just spawned a session scope and/or user service manager, and they aren't ready
+         * yet. We'll call session_create_reply() again once the session scope or the user service manager is
+         * ready, where the function will check again if a reply is then ready to be sent, and then do so if
+         * all is complete - or wait again. */
+        r = session_send_create_reply(session, /* error= */ NULL);
+        if (r < 0)
+                return r;
 
         return 1;
 

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 15f04c4a66aaeedd08511cc9bd732e9f5579d0a9..6da3dd59d5a0057940029117fe79e3a8b1ff5b38 100644 (file)
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -800,7 +800,7 @@ static bool session_ready(Session *s) {
         /* Returns true when the session is ready, i.e. all jobs we enqueued for it are done (regardless if successful or not) */
 
         return !s->scope_job &&
-                !s->user->service_job;
+                (!SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) || !s->user->service_job);
 }
 
 int session_send_create_reply(Session *s, sd_bus_error *error) {

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index c60811f72efb0a1b8d76082e93aaf283287f7341..d8fe018e1da6617a4f7bc3950d93f266f05084a4 100644 (file)
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -722,8 +722,11 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
         assert(s);
         assert(s->user);
 
+        if (!SESSION_CLASS_WANTS_SCOPE(s->class))
+                return 0;
+
         if (!s->scope) {
-                _cleanup_strv_free_ char **after = NULL;
+                _cleanup_strv_free_ char **wants = NULL, **after = NULL;
                 _cleanup_free_ char *scope = NULL;
                 const char *description;
 
@@ -735,6 +738,12 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
 
                 description = strjoina("Session ", s->id, " of User ", s->user->user_record->user_name);
 
+                /* These two have StopWhenUnneeded= set, hence add a dep towards them */
+                wants = strv_new(s->user->runtime_dir_service,
+                                 SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) ? s->user->service : STRV_IGNORE);
+                if (!wants)
+                        return log_oom();
+
                 /* We usually want to order session scopes after systemd-user-sessions.service since the
                  * latter unit is used as login session barrier for unprivileged users. However the barrier
                  * doesn't apply for root as sysadmin should always be able to log in (and without waiting
@@ -754,9 +763,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
                                 &s->leader,
                                 s->user->slice,
                                 description,
-                                /* These two have StopWhenUnneeded= set, hence add a dep towards them */
-                                STRV_MAKE(s->user->runtime_dir_service,
-                                          s->user->service),
+                                wants,
                                 after,
                                 user_record_home_directory(s->user->user_record),
                                 properties,
@@ -1632,11 +1639,13 @@ static const char* const session_type_table[_SESSION_TYPE_MAX] = {
 DEFINE_STRING_TABLE_LOOKUP(session_type, SessionType);
 
 static const char* const session_class_table[_SESSION_CLASS_MAX] = {
-        [SESSION_USER]        = "user",
-        [SESSION_USER_EARLY]  = "user-early",
-        [SESSION_GREETER]     = "greeter",
-        [SESSION_LOCK_SCREEN] = "lock-screen",
-        [SESSION_BACKGROUND]  = "background",
+        [SESSION_USER]          = "user",
+        [SESSION_USER_EARLY]    = "user-early",
+        [SESSION_GREETER]       = "greeter",
+        [SESSION_LOCK_SCREEN]   = "lock-screen",
+        [SESSION_BACKGROUND]    = "background",
+        [SESSION_MANAGER]       = "manager",
+        [SESSION_MANAGER_EARLY] = "manager-early",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(session_class, SessionClass);

diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index a9c44b465f73fe7cf772ccd673ddea088cba2305..2b61f966b5fdc2092b6acc15767828c72c166d60 100644 (file)
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -25,13 +25,24 @@ typedef enum SessionClass {
         SESSION_GREETER,            /* A login greeter pseudo-session */
         SESSION_LOCK_SCREEN,        /* A lock screen */
         SESSION_BACKGROUND,         /* Things like cron jobs, which are non-interactive */
+        SESSION_MANAGER,            /* The service manager */
+        SESSION_MANAGER_EARLY,      /* The service manager for root (which is allowed to run before systemd-user-sessions.service) */
         _SESSION_CLASS_MAX,
         _SESSION_CLASS_INVALID = -EINVAL,
 } SessionClass;
 
-/* Whether we shall allow sessions of this class to run before 'systemd-user-sessions.service'. For now,
- * there's only one class we allow this for. It's generally set for root sessions, but no one else. */
-#define SESSION_CLASS_IS_EARLY(class) ((class) == SESSION_USER_EARLY)
+/* Whether we shall allow sessions of this class to run before 'systemd-user-sessions.service'. It's
+ * generally set for root sessions, but no one else. */
+#define SESSION_CLASS_IS_EARLY(class) IN_SET((class), SESSION_USER_EARLY, SESSION_MANAGER_EARLY)
+
+/* Which session classes want their own scope units? (all of them, except the manager, which comes in its own service unit already */
+#define SESSION_CLASS_WANTS_SCOPE(class) IN_SET((class), SESSION_USER, SESSION_USER_EARLY, SESSION_GREETER, SESSION_LOCK_SCREEN, SESSION_BACKGROUND)
+
+/* Which session classes want their own per-user service manager? */
+#define SESSION_CLASS_WANTS_SERVICE_MANAGER(class) IN_SET((class), SESSION_USER, SESSION_USER_EARLY, SESSION_GREETER, SESSION_LOCK_SCREEN, SESSION_BACKGROUND)
+
+/* Which session classes can pin our user tracking? */
+#define SESSION_CLASS_PIN_USER(class) (!IN_SET((class), SESSION_MANAGER, SESSION_MANAGER_EARLY))
 
 typedef enum SessionType {
         SESSION_UNSPECIFIED,

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 4868eb48fb9f3dc33aa737b4a4f91facb09ee2df..6e41ac6ef60b3bf7f4b1b01d31b85ddda112bd88 100644 (file)
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -336,7 +336,17 @@ int user_load(User *u) {
         return 0;
 }
 
-static void user_start_service(User *u) {
+static bool user_wants_service_manager(User *u) {
+        assert(u);
+
+        LIST_FOREACH(sessions_by_user, s, u->sessions)
+                if (SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class))
+                        return true;
+
+        return false;
+}
+
+void user_start_service_manager(User *u) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         int r;
 
@@ -346,6 +356,12 @@ static void user_start_service(User *u) {
          * start the per-user slice or the systemd-runtime-dir@.service instance, as those are pulled in both by
          * user@.service and the session scopes as dependencies. */
 
+        if (u->stopping) /* Don't try to start this if the user is going down */
+                return;
+
+        if (!user_wants_service_manager(u)) /* Only start user service manager if there's at least one session which wants it */
+                return;
+
         u->service_job = mfree(u->service_job);
 
         r = manager_start_unit(u->manager, u->service, &error, &u->service_job);
@@ -448,7 +464,7 @@ int user_start(User *u) {
         u->stopping = false;
 
         if (!u->started)
-                log_debug("Starting services for new user %s.", u->user_record->user_name);
+                log_debug("Tracking new user %s.", u->user_record->user_name);
 
         /* Save the user data so far, because pam_systemd will read the XDG_RUNTIME_DIR out of it while starting up
          * systemd --user.  We need to do user_save_internal() because we have not "officially" started yet. */
@@ -458,7 +474,7 @@ int user_start(User *u) {
         (void) user_update_slice(u);
 
         /* Start user@UID.service */
-        user_start_service(u);
+        user_start_service_manager(u);
 
         if (!u->started) {
                 if (!dual_timestamp_is_set(&u->timestamp))
@@ -651,6 +667,19 @@ static usec_t user_get_stop_delay(User *u) {
         return u->manager->user_stop_delay;
 }
 
+static bool user_pinned_by_sessions(User *u) {
+        assert(u);
+
+        /* Returns true if at least one session exists that shall keep the user tracking alive. That
+         * generally means one session that isn't the service manager still exists. */
+
+        LIST_FOREACH(sessions_by_user, i, u->sessions)
+                if (SESSION_CLASS_PIN_USER(i->class))
+                        return true;
+
+        return false;
+}
+
 bool user_may_gc(User *u, bool drop_not_started) {
         int r;
 
@@ -659,7 +688,7 @@ bool user_may_gc(User *u, bool drop_not_started) {
         if (drop_not_started && !u->started)
                 return true;
 
-        if (u->sessions)
+        if (user_pinned_by_sessions(u))
                 return false;
 
         if (u->last_session_timestamp != USEC_INFINITY) {
@@ -718,22 +747,26 @@ UserState user_get_state(User *u) {
         if (!u->started || u->service_job)
                 return USER_OPENING;
 
-        if (u->sessions) {
-                bool all_closing = true;
+        bool any = false, all_closing = true;
+        LIST_FOREACH(sessions_by_user, i, u->sessions) {
+                SessionState state;
 
-                LIST_FOREACH(sessions_by_user, i, u->sessions) {
-                        SessionState state;
+                /* Ignore sessions that don't pin the user, i.e. are not supposed to have an effect on user state */
+                if (!SESSION_CLASS_PIN_USER(i->class))
+                        continue;
 
-                        state = session_get_state(i);
-                        if (state == SESSION_ACTIVE)
-                                return USER_ACTIVE;
-                        if (state != SESSION_CLOSING)
-                                all_closing = false;
-                }
+                state = session_get_state(i);
+                if (state == SESSION_ACTIVE)
+                        return USER_ACTIVE;
+                if (state != SESSION_CLOSING)
+                        all_closing = false;
 
-                return all_closing ? USER_CLOSING : USER_ONLINE;
+                any = true;
         }
 
+        if (any)
+                return all_closing ? USER_CLOSING : USER_ONLINE;
+
         if (user_check_linger_file(u) > 0 && user_unit_active(u))
                 return USER_LINGERING;
 
@@ -828,7 +861,7 @@ void user_update_last_session_timer(User *u) {
 
         assert(u);
 
-        if (u->sessions) {
+        if (user_pinned_by_sessions(u)) {
                 /* There are sessions, turn off the timer */
                 u->last_session_timestamp = USEC_INFINITY;
                 u->timer_event_source = sd_event_source_unref(u->timer_event_source);

diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 21b9f8f348ec5d73b1eecbfd534ef78e783e1a70..7108520f6593f7aa6b4d869a1d9e48a1d11e4db6 100644 (file)
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -57,6 +57,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free);
 
 bool user_may_gc(User *u, bool drop_not_started);
 void user_add_to_gc_queue(User *u);
+void user_start_service_manager(User *u);
 int user_start(User *u);
 int user_stop(User *u, bool force);
 int user_finalize(User *u);