logind: rework GC logic

In logind we generally want to stop user@.service for a user once they
log out. So the usual rule is that whenever a User object is around that
has no pinning sessions we should close it.

Except that it isn't that easy. We allow that user@.service is also
manually started, in which case the User object is created but not
pinned by any session.

Let's rework how this is handled: we define two different GC modes. In
one GC mode we'll keep the User object around whenever *any* session
exists (thus: including the user@.service session), and one where we
only keep it around whenever a *pinning* session exists (i.e. when a
user actually logs in, but the user@.service session doesn't count like
that).

And the trick is now that we start out in the *any* GC mode, and switch
to the *pinning* GC mode once the first user session logs in.

This should make things more robust as we know exactly in which state we
are and when to GC a user.

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 31efb0d113dbb020d8e5a22ad0bccc09fdaf5639..99b9da6ba10802795c7118e347ef0e7a627094a1 100644 (file)
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -937,9 +937,17 @@ static int create_session(
                 goto fail;
 
         session->original_type = session->type = t;
-        session->class = c;
         session->remote = remote;
         session->vtnr = vtnr;
+        session->class = c;
+
+        /* Once the first session that is of a pinning class shows up we'll change the GC mode for the user
+         * from USER_GC_BY_ANY to USER_GC_BY_PIN, so that the user goes away once the last pinning session
+         * goes away. Background: we want that user@.service – when started manually – remains around (which
+         * itself is a non-pinning session), but gets stopped when the last pinning session goes away. */
+
+        if (SESSION_CLASS_PIN_USER(c))
+                user->gc_mode = USER_GC_BY_PIN;
 
         if (!isempty(tty)) {
                 session->tty = strdup(tty);

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 6de7c47c7a77d67c25649d193e333ea99905a0d0..e6e57ad79ee41ac5c529706b1c5e04c176ad0061 100644 (file)
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -63,6 +63,7 @@ int user_new(User **ret,
                 .manager = m,
                 .user_record = user_record_ref(ur),
                 .last_session_timestamp = USEC_INFINITY,
+                .gc_mode = USER_GC_BY_ANY,
         };
 
         if (asprintf(&u->state_file, "/run/systemd/users/" UID_FMT, ur->uid) < 0)
@@ -162,10 +163,12 @@ static int user_save_internal(User *u) {
                 "# This is private data. Do not parse.\n"
                 "NAME=%s\n"
                 "STATE=%s\n"         /* friendly user-facing state */
-                "STOPPING=%s\n",     /* low-level state */
+                "STOPPING=%s\n"      /* low-level state */
+                "GC_MODE=%s\n",
                 u->user_record->user_name,
                 user_state_to_string(user_get_state(u)),
-                yes_no(u->stopping));
+                yes_no(u->stopping),
+                user_gc_mode_to_string(u->gc_mode));
 
         /* LEGACY: no-one reads RUNTIME= anymore, drop it at some point */
         if (u->runtime_path)
@@ -302,7 +305,7 @@ int user_save(User *u) {
 }
 
 int user_load(User *u) {
-        _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *stopping = NULL, *last_session_timestamp = NULL;
+        _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *stopping = NULL, *last_session_timestamp = NULL, *gc_mode = NULL;
         int r;
 
         assert(u);
@@ -312,7 +315,8 @@ int user_load(User *u) {
                            "STOPPING",               &stopping,
                            "REALTIME",               &realtime,
                            "MONOTONIC",              &monotonic,
-                           "LAST_SESSION_TIMESTAMP", &last_session_timestamp);
+                           "LAST_SESSION_TIMESTAMP", &last_session_timestamp,
+                           "GC_MODE",                &gc_mode);
         if (r == -ENOENT)
                 return 0;
         if (r < 0)
@@ -333,6 +337,10 @@ int user_load(User *u) {
         if (last_session_timestamp)
                 (void) deserialize_usec(last_session_timestamp, &u->last_session_timestamp);
 
+        u->gc_mode = user_gc_mode_from_string(gc_mode);
+        if (u->gc_mode < 0)
+                u->gc_mode = USER_GC_BY_PIN;
+
         return 0;
 }
 
@@ -676,11 +684,21 @@ static bool user_pinned_by_sessions(User *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;
+        switch (u->gc_mode) {
 
-        return false;
+        case USER_GC_BY_ANY:
+                return u->sessions;
+
+        case USER_GC_BY_PIN:
+                LIST_FOREACH(sessions_by_user, i, u->sessions)
+                        if (SESSION_CLASS_PIN_USER(i->class))
+                                return true;
+
+                return false;
+
+        default:
+                assert_not_reached();
+        }
 }
 
 bool user_may_gc(User *u, bool drop_not_started) {
@@ -912,6 +930,13 @@ static const char* const user_state_table[_USER_STATE_MAX] = {
 
 DEFINE_STRING_TABLE_LOOKUP(user_state, UserState);
 
+static const char* const user_gc_mode_table[_USER_GC_MODE_MAX] = {
+        [USER_GC_BY_PIN] = "pin",
+        [USER_GC_BY_ANY] = "any",
+};
+
+DEFINE_STRING_TABLE_LOOKUP(user_gc_mode, UserGCMode);
+
 int config_parse_tmpfs_size(
                 const char* unit,
                 const char *filename,

diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 7108520f6593f7aa6b4d869a1d9e48a1d11e4db6..9bda5dde4218139daf25bbbb690426392f380a76 100644 (file)
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -19,6 +19,13 @@ typedef enum UserState {
         _USER_STATE_INVALID = -EINVAL,
 } UserState;
 
+typedef enum UserGCMode {
+        USER_GC_BY_ANY,         /* any session pins this user */
+        USER_GC_BY_PIN,         /* only sessions with an explicitly pinning class pin this user */
+        _USER_GC_MODE_MAX,
+        _USER_GC_MODE_INVALID = -EINVAL,
+} UserGCMode;
+
 struct User {
         Manager *manager;
 
@@ -41,6 +48,7 @@ struct User {
         /* Set up when the last session of the user logs out */
         sd_event_source *timer_event_source;
 
+        UserGCMode gc_mode;
         bool in_gc_queue:1;
 
         bool started:1;       /* Whenever the user being started, has been started or is being stopped again. */
@@ -73,4 +81,7 @@ void user_update_last_session_timer(User *u);
 const char* user_state_to_string(UserState s) _const_;
 UserState user_state_from_string(const char *s) _pure_;
 
+const char* user_gc_mode_to_string(UserGCMode m) _const_;
+UserGCMode user_gc_mode_from_string(const char *s) _pure_;
+
 CONFIG_PARSER_PROTOTYPE(config_parse_compat_user_tasks_max);