Merge pull request #23396 from msekletar/fix-idle-action-lock

logind: remember our idle state and use it to detect idle level transitions

src/login/logind.c

@@ -963,18 +963,33 @@ static int manager_dispatch_idle_action(sd_event_source *s, uint64_t t, void *us
         n = now(CLOCK_MONOTONIC);
 
         r = manager_get_idle_hint(m, &since);
-        if (r <= 0)
+        if (r <= 0) {
                 /* Not idle. Let's check if after a timeout it might be idle then. */
                 elapse = n + m->idle_action_usec;
-        else {
+                m->was_idle = false;
+        } else {
+
                 /* Idle! Let's see if it's time to do something, or if
                  * we shall sleep for longer. */
 
                 if (n >= since.monotonic + m->idle_action_usec &&
                     (m->idle_action_not_before_usec <= 0 || n >= m->idle_action_not_before_usec + m->idle_action_usec)) {
-                        log_info("System idle. Will %s now.", handle_action_verb_to_string(m->idle_action));
+                        bool is_edge = false;
 
-                        manager_handle_action(m, 0, m->idle_action, false, false);
+                        /* We weren't idle previously or some activity happened while we were sleeping, and now we are
+                         * idle. Let's remember that for the next time and make this an edge transition. */
+                        if (!m->was_idle || since.monotonic >= m->idle_action_not_before_usec) {
+                                is_edge = true;
+                                m->was_idle = true;
+                        }
+
+                        if (m->idle_action == HANDLE_LOCK && !is_edge)
+                                /* We are idle and we were before so we are actually not taking any action. */
+                                log_debug("System idle.");
+                        else
+                                log_info("System idle. Will %s now.", handle_action_verb_to_string(m->idle_action));
+
+                        manager_handle_action(m, 0, m->idle_action, false, is_edge);
                         m->idle_action_not_before_usec = n;
                 }
 

src/login/logind.h

@@ -94,6 +94,7 @@ struct Manager {
         usec_t idle_action_usec;
         usec_t idle_action_not_before_usec;
         HandleAction idle_action;
+        bool was_idle;
 
         HandleAction handle_power_key;
         HandleAction handle_power_key_long_press;

test/test-functions

@@ -1737,6 +1737,13 @@ install_basic_tools() {
     image_install -o ldconfig.real
     # for TEST-35-LOGIN
     image_install -o evemu-device evemu-event
+    if command -v expect >/dev/null && command -v tclsh >/dev/null ; then
+        # shellcheck disable=SC2016
+        version="$(tclsh <<< 'puts $tcl_version')"
+
+        image_install expect
+        inst_recursive /usr/lib64/tcl"$version" /usr/share/tcl"$version"
+    fi
 }
 
 install_debug_tools() {

test/units/testsuite-35.sh

@@ -360,6 +360,89 @@ EOF
     fi
 }
 
+setup_idle_action_lock() {
+    useradd testuser ||:
+
+    mkdir -p /run/systemd/logind.conf.d/
+    cat >/run/systemd/logind.conf.d/idle-action-lock.conf <<EOF
+[Login]
+IdleAction=lock
+IdleActionSec=1s
+EOF
+    systemctl restart systemd-logind.service
+    systemctl service-log-level systemd-logind.service debug
+}
+
+teardown_idle_action_lock() {(
+    set +ex
+    rm -f /run/systemd/logind.conf.d/idle-action-lock.conf
+    pkill -9 -u "$(id -u testuser)"
+    userdel -r testuser
+    systemctl restart systemd-logind.service
+)}
+
+test_lock_idle_action() {
+    if ! command -v expect >/dev/null ; then
+        echo >&2 "expect not installed, skiping test ${FUNCNAME[0]}"
+        return 0
+    fi
+
+    setup_idle_action_lock
+    trap teardown_idle_action_lock RETURN
+
+    if loginctl --no-legend | awk '{ print $3; }' | sort -u | grep -q testuser ; then
+        echo >&2 "Session of the \'testuser\' is already present."
+        return 1
+    fi
+
+    # IdleActionSec is set 1s but the accuracy of associated timer is 30s so we
+    # need to sleep in worst case for 31s to make sure timer elapsed. We sleep
+    # here for 35s to accomodate for any possible scheudling delays.
+    cat > /tmp/test.exp <<EOF
+spawn systemd-run -G -t -p PAMName=login -p User=testuser bash
+send "sleep 35\r"
+send "echo foobar\r"
+send "sleep 35\r"
+send "exit\r"
+interact
+wait
+EOF
+
+    ts="$(date '+%H:%M:%S')"
+    busctl --match "type='signal',sender='org.freedesktop.login1',interface='org.freedesktop.login1.Session',member='Lock'" monitor  > dbus.log &
+
+    expect /tmp/test.exp &
+
+    # Sleep a bit to give expect time to spawn systemd-run before we check for
+    # the presence of resulting session.
+    sleep 1
+    if [ "$(loginctl --no-legend | awk '{ print $3; }' | sort -u | grep -c testuser)" != 1 ] ; then
+        echo >&2 "\'testuser\' is expected to have exactly one session running."
+        return 1
+    fi
+
+    wait %2
+    kill %1
+
+    # We slept for 35s , in that interval all sessions should have become idle
+    # and "Lock" signal should have been sent out. Then we wrote to tty to make
+    # session active again and next we slept for another 35s so sessions have
+    # become idle again. 'Lock' signal is sent out for each session, we have at
+    # least one session, so minimum of 2 "Lock" signals must have been sent.
+    if [ "$(grep -c Member=Lock dbus.log)" -lt 2 ]; then
+        echo >&2 "Too few 'Lock' D-Bus signal sent, expected at least 2."
+        return 1
+    fi
+
+    journalctl -b -u systemd-logind.service --since="$ts" > logind.log
+    if [ "$(grep -c 'System idle. Will be locked now.' logind.log)" -lt 2 ]; then
+        echo >&2 "System haven't entered idle state at least 2 times."
+        return 1
+    fi
+
+    rm -f dbus.log logind.log
+}
+
 : >/failed
 
 test_enable_debug
@@ -368,6 +451,7 @@ test_started
 test_suspend_on_lid
 test_shutdown
 test_session
+test_lock_idle_action
 
 touch /testok
 rm /failed