Merge pull request #23999 from msekletar/revert-background-session-no-user-instance

Revert of recent changes in handling of background sessions

NEWS

@@ -1,13 +1,5 @@
 systemd System and Service Manager
 
-CHANGES WITH 252:
-        Backwards-incompatible changes:
-
-        * systemd-logind's background session class, which is by default used
-          for sessions started via cron, is no longer going to start systemd
-          --user instance. Note that because of that the user dbus instance
-          will not start either. Other session types remain unaffected.
-
 CHANGES WITH 251:
 
         Backwards-incompatible changes:

src/core/execute.c

@@ -1213,15 +1213,13 @@ static int setup_pam(
 
                 if (getttyname_malloc(STDIN_FILENO, &q) >= 0)
                         tty = strjoina("/dev/", q);
-                else
-                        /* If everything else failed then let's just use value "systemd". This will cause that session
-                         * isn't going to be marked as "background" and user manager will be started. */
-                        tty = "systemd";
         }
 
-        pam_code = pam_set_item(handle, PAM_TTY, tty);
-        if (pam_code != PAM_SUCCESS)
-               goto fail;
+        if (tty) {
+                pam_code = pam_set_item(handle, PAM_TTY, tty);
+                if (pam_code != PAM_SUCCESS)
+                        goto fail;
+        }
 
         STRV_FOREACH(nv, *env) {
                 pam_code = pam_putenv(handle, *nv);

src/login/logind-dbus.c

@@ -1305,7 +1305,7 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu
                         return r;
 
                 if (manager_add_user_by_uid(m, uid, &u) >= 0)
-                        user_start(u, /* want_user_instance= */ true);
+                        user_start(u);
 
         } else {
                 User *u;

src/login/logind-session.c

@@ -667,7 +667,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
                 after = strv_new("systemd-logind.service",
                                  s->user->runtime_dir_service,
                                  !uid_is_system(s->user->user_record->uid) ? "systemd-user-sessions.service" : STRV_IGNORE,
-                                 s->class != SESSION_BACKGROUND ? s->user->service : STRV_IGNORE);
+                                 s->user->service);
                 if (!after)
                         return log_oom();
 
@@ -679,7 +679,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
                                 description,
                                 /* These two have StopWhenUnneeded= set, hence add a dep towards them */
                                 STRV_MAKE(s->user->runtime_dir_service,
-                                          s->class != SESSION_BACKGROUND ? s->user->service : NULL),
+                                          s->user->service),
                                 after,
                                 user_record_home_directory(s->user->user_record),
                                 properties,
@@ -711,7 +711,7 @@ int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error) {
         if (s->started)
                 return 0;
 
-        r = user_start(s->user, /* want_user_instance= */ s->class != SESSION_BACKGROUND);
+        r = user_start(s->user);
         if (r < 0)
                 return r;
 

src/login/logind-user.c

@@ -442,7 +442,7 @@ static int user_update_slice(User *u) {
         return 0;
 }
 
-int user_start(User *u, bool want_user_instance) {
+int user_start(User *u) {
         assert(u);
 
         if (u->started && !u->stopping)
@@ -465,8 +465,7 @@ int user_start(User *u, bool want_user_instance) {
         (void) user_update_slice(u);
 
         /* Start user@UID.service */
-        if (want_user_instance)
-                user_start_service(u);
+        user_start_service(u);
 
         if (!u->started) {
                 if (!dual_timestamp_is_set(&u->timestamp))

src/login/logind-user.h

@@ -57,7 +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);
-int user_start(User *u, bool need_user_instance);
+int user_start(User *u);
 int user_stop(User *u, bool force);
 int user_finalize(User *u);
 UserState user_get_state(User *u);

src/login/logind.c

@@ -1041,6 +1041,7 @@ static int manager_startup(Manager *m) {
         int r;
         Seat *seat;
         Session *session;
+        User *user;
         Button *button;
         Inhibitor *inhibitor;
 
@@ -1117,7 +1118,9 @@ static int manager_startup(Manager *m) {
         HASHMAP_FOREACH(seat, m->seats)
                 (void) seat_start(seat);
 
-        /* Users are started by respective sessions */
+        HASHMAP_FOREACH(user, m->users)
+                (void) user_start(user);
+
         HASHMAP_FOREACH(session, m->sessions)
                 (void) session_start(session, NULL, NULL);
 

src/login/pam_systemd.c

@@ -790,10 +790,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
                              * does the PAM session registration early for new connections, and registers a pty only
                              * much later (this is because it doesn't know yet if it needs one at all, as whether to
                              * register a pty or not is negotiated much later in the protocol). */
-        } else if (streq(tty, "systemd")) {
-                if (isempty(class))
-                        class = "user";
-                tty = NULL;
+
         } else
                 /* Chop off leading /dev prefix that some clients specify, but others do not. */
                 tty = skip_dev_prefix(tty);

test/TEST-35-LOGIN/test.sh

@@ -8,7 +8,7 @@ TEST_DESCRIPTION="Tests for systemd-logind"
 . "${TEST_BASE_DIR:?}/test-functions"
 
 test_append_files() {
-    image_install -o evemu-device evemu-event crond crontab
+    image_install -o evemu-device evemu-event
 }
 
 do_test "$@"

test/units/testsuite-35.sh

@@ -430,50 +430,6 @@ EOF
     fi
 }
 
-teardown_cron() (
-    set +ex
-
-    pkill -u "$(id -u logind-test-user)"
-    sleep 1
-    pkill -KILL -u "$(id -u logind-test-user)"
-    pkill crond
-    crontab -r -u logind-test-user
-
-    return 0
-)
-
-test_no_user_instance_for_cron() {
-    if ! command -v crond || ! command -v crontab ; then
-        echo "Skipping test for background cron sessions because cron is missing."
-        return
-    fi
-
-    trap teardown_cron RETURN
-
-    # Setup cron
-    crond -s -n &
-    # Install crontab for the test user that runs sleep every minute. But let's sleep for
-    # 65 seconds to make sure there is overlap between two consecutive runs, i.e. we have
-    # always a cron session running.
-    crontab -u logind-test-user - <<EOF
-RANDOM_DELAY=0
-* * * * * /bin/sleep 65
-EOF
-
-    # Let's wait (at most one interval plus 10s to accommodate for slow machines) for at least one session
-    # of the test user
-    timeout 70 bash -c "while ! loginctl --no-legend list-sessions | grep -q logind-test-user; do sleep 1; done"
-
-    # Check that all sessions of test user have class=background and no user instance was started
-    # for the test user.
-    while read -r s _; do
-        assert_eq "$(loginctl --property Class --value show-session "$s")" "background"
-    done < <(loginctl --no-legend list-sessions | grep logind-test-user)
-
-    assert_eq "$(systemctl --property ActiveState --value show user@"$(id -u logind-test-user)".service)" "inactive"
-    assert_eq "$(systemctl --property SubState --value show user@"$(id -u logind-test-user)".service)" "dead"
-}
-
 test_session_properties() {
     local s
 
@@ -499,7 +455,6 @@ test_suspend_on_lid
 test_shutdown
 test_session
 test_lock_idle_action
-test_no_user_instance_for_cron
 test_session_properties
 
 touch /testok