logind: rework how we manage the slice and user-runtime-dir@.service unit for each user

Instead of managing it explicitly, let's simplify things and rely on
regular Wants=/Requires= dependencies to pull in these units from
user@.service and the session scope, and StopWhenUneeded= to stop these
auxiliary units again. This way, they can be pulled in easily by
unrelated units too.

This simplifies things quite a bit: for each session we now only need to
manage the session scope, and for each user the user@.service, the other
units are not something we need to manage anymore.

This patch also makes sure that if user@.service of a user is masked we
will continue to work, and user-runtime-dir@.service will still be
correctly pulled in, as it is now a dependency of the scope unit.

Fixes: #9461
Replaces: #5546
(cherry picked from commit 25a1ab4ed48b72e974f77a68dcbe3521014787bb)

1 files changed, 34 insertions, 22 deletions

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 45b25aab0..2c4539398 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -27,6 +27,7 @@
 #include "path-util.h"
 //#include "process-util.h"
 #include "string-table.h"
+//#include "strv.h"
 #include "terminal-util.h"
 #include "user-util.h"
 #include "util.h"
@@ -571,18 +572,18 @@ int session_activate(Session *s) {
 }
 
 #if 0 /// UNNEEDED by elogind
-static int session_start_scope(Session *s, sd_bus_message *properties) {
+static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_error *error) {
         int r;
 
         assert(s);
         assert(s->user);
 
         if (!s->scope) {
-                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
                 _cleanup_free_ char *scope = NULL;
-                char *job = NULL;
                 const char *description;
 
+                s->scope_job = mfree(s->scope_job);
+
                 scope = strjoin("session-", s->id, ".scope");
                 if (!scope)
                         return log_oom();
@@ -595,17 +596,15 @@ static int session_start_scope(Session *s, sd_bus_message *properties) {
                                 s->leader,
                                 s->user->slice,
                                 description,
-                                "systemd-logind.service",
-                                "systemd-user-sessions.service",
+                                STRV_MAKE(s->user->runtime_dir_service, s->user->service), /* These two have StopWhenUnneeded= set, hence add a dep towards them */
+                                STRV_MAKE("systemd-logind.service", "systemd-user-sessions.service", s->user->runtime_dir_service, s->user->service), /* And order us after some more */
                                 properties,
-                                &error,
-                                &job);
+                                error,
+                                &s->scope_job);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to start session scope %s: %s", scope, bus_error_message(&error, r));
-
+                        return log_error_errno(r, "Failed to start session scope %s: %s", scope, bus_error_message(error, r));
 
                 s->scope = TAKE_PTR(scope);
-                free_and_replace(s->scope_job, job);
         }
 
         if (s->scope)
@@ -613,6 +612,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties) {
 
         return 0;
 }
+int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error) {
 #else
 static int session_start_cgroup(Session *s) {
         int r;
@@ -633,8 +633,6 @@ static int session_start_cgroup(Session *s) {
         return 0;
 }
 #endif // 0
-
-int session_start(Session *s, sd_bus_message *properties) {
         int r;
 
         assert(s);
@@ -642,6 +640,9 @@ int session_start(Session *s, sd_bus_message *properties) {
         if (!s->user)
                 return -ESTALE;
 
+        if (s->stopping)
+                return -EINVAL;
+
         if (s->started)
                 return 0;
 
@@ -649,9 +650,8 @@ int session_start(Session *s, sd_bus_message *properties) {
         if (r < 0)
                 return r;
 
-        /* Create cgroup */
 #if 0 /// elogind does its own session management
-        r = session_start_scope(s, properties);
+        r = session_start_scope(s, properties, error);
 #else
         r = session_start_cgroup(s);
 #endif // 0
@@ -706,21 +706,24 @@ static int session_stop_scope(Session *s, bool force) {
          * that is left in the scope is "left-over". Informing systemd about this has the benefit that it will log
          * when killing any processes left after this point. */
         r = manager_abandon_scope(s->manager, s->scope, &error);
-        if (r < 0)
+        if (r < 0) {
                 log_warning_errno(r, "Failed to abandon session scope, ignoring: %s", bus_error_message(&error, r));
+                sd_bus_error_free(&error);
+        }
+
+        s->scope_job = mfree(s->scope_job);
 
         /* Optionally, let's kill everything that's left now. */
         if (force || manager_shall_kill(s->manager, s->user->name)) {
-                char *job = NULL;
 
-                r = manager_stop_unit(s->manager, s->scope, &error, &job);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to stop session scope: %s", bus_error_message(&error, r));
+                r = manager_stop_unit(s->manager, s->scope, &error, &s->scope_job);
+                if (r < 0) {
+                        if (force)
+                                return log_error_errno(r, "Failed to stop session scope: %s", bus_error_message(&error, r));
 
-                free(s->scope_job);
-                s->scope_job = job;
+                        log_warning_errno(r, "Failed to stop session scope, ignoring: %s", bus_error_message(&error, r));
+                }
         } else {
-                s->scope_job = mfree(s->scope_job);
 
                 /* With no killing, this session is allowed to persist in "closing" state indefinitely.
                  * Therefore session stop and session removal may be two distinct events.
@@ -758,8 +761,17 @@ int session_stop(Session *s, bool force) {
 
         assert(s);
 
+        /* This is called whenever we begin with tearing down a session record. It's called in four cases: explicit API
+         * request via the bus (either directly for the session object or for the seat or user object this session
+         * belongs to; 'force' is true), or due to automatic GC (i.e. scope vanished; 'force' is false), or because the
+         * session FIFO saw an EOF ('force' is false), or because the release timer hit ('force' is false). */
+
         if (!s->user)
                 return -ESTALE;
+        if (!s->started)
+                return 0;
+        if (s->stopping)
+                return 0;
 
         s->timer_event_source = sd_event_source_unref(s->timer_event_source);