timer: avoid re-arming WakeSystem=yes timer after suspend

There's an edge case where a single-shot timer using WakeSystem=yes
could get re-armed after elapsing. For example, when a timer is created
using:

$ systemd-run --user --on-active="1m" --timer-property=WakeSystem=yes flatpak run io.bassi.Amberol

and the system is then suspended, following sequence of events may
happen:

Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Installed new job run-p192456-i199160.timer/start as 7675
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Enqueued job run-p192456-i199160.timer/start as 7675
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 59.999999s.
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Changed dead -> waiting
Jul 08 06:57:25 systemd[2640]: run-p192456-i199160.timer: Job 7675 run-p192456-i199160.timer/start finished, result=done
Jul 08 06:57:25 systemd[2640]: Started [systemd-run] /usr/bin/flatpak run io.bassi.Amberol.
Jul 08 06:58:13 systemd[2640]: run-p192456-i199160.timer: Time change, recalculating next elapse.
Jul 08 06:58:13 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 12.785674s.
Jul 08 06:58:26 systemd[2640]: run-p192456-i199160.timer: Timer elapsed.
Jul 08 06:58:26 systemd[2640]: run-p192456-i199160.timer: Changed waiting -> running
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Got notified about unit deactivation.
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Monotonic timer elapses in 33.544681s.
Jul 08 06:58:29 systemd[2640]: run-p192456-i199160.timer: Changed running -> waiting
Jul 08 06:59:48 systemd[2640]: run-p192456-i199160.timer: Timer elapsed.

1) The timer is armed at 06:57:25. timer_enter_waiting()
   calculates following values:

   base = M0 (current CLOCK_MONOTONIC)
   usec_shift_clock(base, CLOCK_MONOTONIC, CLOCK_BOOTTIME_ALARM) = M0 (no delta yet)
   v->next_elapse = M0 + 60s

   The timer is on CLOCK_BOOTTIME_ALARM at M0 + 60s

2) System suspends. If the system ran for ~12s and was suspended for
   ~36s, at resume we'd get:

   CLOCK_MONOTONIC = M0 + 12s
   CLOCK_BOOTTIME = M0 + 12s + 36s = M0 + 48s

3) timer_time_change() fires and calls timer_enter_waiting(t, true)

   Because time_change=true, v->next_elapse is not recalculated and
   keeps the original M0 + 60s value. The timer then correctly computes
   the remaining time as ~12.8 seconds:

        Jul 08 06:58:13 ...: Monotonic timer elapses in 12.785674s.

4) Timer elapses at 06:58:26

        Jul 08 06:58:26 ...: Timer elapsed.

   timer_enter_running() sets last_trigger.monotonic to CLOCK_MONOTONIC
   (which equals to M0 + 12s before suspend + 13s after suspend, thus
   + 25s)

5) Unit deactivates at 06:58:29

   timer_trigger_notify() calls timer_enter_waiting(t, false) -
   time_change=false; that means that this time v->next_elapse gets
   recalculated:

   base = inactive_exit_timestamp.monotonic = M0 (i.e. when the timer was originally armed)
   usec_shift_clock(M0, CLOCK_MONOTONIC, CLOCK_BOOTTIME_ALARM)
        a = now(CLOCK_MONOTONIC) = M0 + 28s
        b = now(CLOCK_BOOTTIME_ALARM) = M0 + 64s
        result = b - (a - M0) = (M0 + 64s) - 28s = M0 + 36s => the time spent in suspend

   Hence v->next_elapse = (M0 + 36s) + 60s = M0 + 96s

   This then skips disabling of one-shot timers in the following check,
   because the expression

        v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t))

   is false, because the v->next_elapse (M0 + 96s) is not less than the
   current time (M0 + 64s), so the timer is re-armed again:

        Jul 08 06:58:29 ...: Monotonic timer elapses in 33.544681s

Let's mitigate this by skipping the next elapse timestamp recalculations
for one-shot timers for which we've already calculated the value in this
activation cycle.

Resolves: #42929

diff --git a/src/core/timer.c b/src/core/timer.c
index 510d8e199577420d723ffb604feae6e07c4ae196..5716c4f81d566f0e5c3a60e23a567c9f1376b206 100644 (file)
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -498,16 +498,23 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                                 assert_not_reached();
                         }
 
-                        if (!time_change)
-                                v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
-
-                        if (dual_timestamp_is_set(&t->last_trigger) &&
-                            !time_change &&
-                            v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t)) &&
-                            IN_SET(v->base, TIMER_ACTIVE, TIMER_BOOT, TIMER_STARTUP)) {
-                                /* This is a one time trigger, disable it now */
-                                v->disabled = true;
-                                continue;
+                        if (!time_change) {
+                                bool is_oneshot = IN_SET(v->base, TIMER_ACTIVE, TIMER_BOOT, TIMER_STARTUP);
+
+                                /* Skip recalculating the next elapse timestamp for one-shot timers for which
+                                 * we've already calculated the value in this activation cycle. */
+                                if (!(v->next_elapse > 0 &&
+                                      t->last_trigger.monotonic > base &&
+                                      is_oneshot))
+                                        v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
+
+                                if (dual_timestamp_is_set(&t->last_trigger) &&
+                                    v->next_elapse < triple_timestamp_by_clock(&ts, TIMER_MONOTONIC_CLOCK(t)) &&
+                                    is_oneshot) {
+                                        /* This is a one time trigger, disable it now. */
+                                        v->disabled = true;
+                                        continue;
+                                }
                         }
 
                         if (!found_monotonic)