From: Lennart Poettering Date: Wed, 29 Nov 2023 10:24:10 +0000 (+0100) Subject: logind: rework GC logic X-Git-Tag: v256-rc1~1186^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d2a4c37972494aaf3deea5bc971bc45ab8f73f25;p=thirdparty%2Fsystemd.git 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 31efb0d113d..99b9da6ba10 100644 --- 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 6de7c47c7a7..e6e57ad79ee 100644 --- 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 7108520f659..9bda5dde421 100644 --- 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);