core/timer: fix next trigger with RandomizedOffsetSec + Persistent (#42826)

When a calendar timer with RandomizedOffsetSec and Persistent=true fires a
catch-up activation, last_trigger is set to the current wall-clock time.
This inherently already includes any randomized offset, because the
trigger was that the activation time including the offset had passed.

When computing the next elapse, calendar_spec_next_usec() finds the next
calendar boundary after the base time, and then random_offset is added to the
result. If the base time already includes the offset, the next calendar
boundary is one period too far in the future, causing a scheduled activation
to be skipped.

Fix this by always subtracting random_offset from the base time before passing
it to calendar_spec_next_usec(), matching what the fallback branches (using
inactive_exit_timestamp or current time) already do. This puts the base into
"pre-offset calendar space" so that the next calendar match and subsequent
offset addition yield the correct next activation time.

Fixes #42337.

diff --git a/src/core/timer.c b/src/core/timer.c
index 70bcb56b83de3f72acc62b9e788d433e6f8ac7c4..441492dd159ba49b028acdc188baae2977a03700 100644 (file)
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -402,12 +402,7 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                         /* If DeferReactivation= is enabled, schedule the job based on the last time
                          * the trigger unit entered inactivity. Otherwise, if we know the last time
                          * this was triggered, schedule the job based relative to that. If we don't,
-                         * just start from the activation time or realtime.
-                         *
-                         * Unless we have a real last-trigger time, we subtract the random_offset because
-                         * any event that elapsed within the last random_offset has actually been delayed
-                         * and thus hasn't truly elapsed yet. */
-
+                         * just start from the activation time or realtime. */
                         if (t->defer_reactivation &&
                             dual_timestamp_is_set(&trigger->inactive_enter_timestamp)) {
                                 if (dual_timestamp_is_set(&t->last_trigger))
@@ -425,9 +420,9 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                                 if (t->last_trigger.monotonic < boot_monotonic)
                                         rebase_after_boot_time = true;
                         } else if (dual_timestamp_is_set(&UNIT(t)->inactive_exit_timestamp))
-                                b = UNIT(t)->inactive_exit_timestamp.realtime - random_offset;
+                                b = UNIT(t)->inactive_exit_timestamp.realtime;
                         else {
-                                b = ts.realtime - random_offset;
+                                b = ts.realtime;
                                 rebase_after_boot_time = true;
                         }
 
@@ -438,14 +433,21 @@ static void timer_enter_waiting(Timer *t, bool time_change) {
                                  * base to calendar_spec_next_usec() would schedule the next elapse relative to
                                  * that stale time instead of now, so systemctl list-timers keeps showing the
                                  * pre-adjustment elapse and the timer never catches up (see #6036). Clamp to
-                                 * now (minus the random offset, matching the "start from now" branch above) so
-                                 * we recalculate from the current time. */
+                                 * now so we recalculate from the current time. */
                                 log_unit_debug(UNIT(t),
                                                "Calendar timer base time %s is in the future, recalculating from the current time.",
                                                FORMAT_TIMESTAMP(b));
-                                b = ts.realtime - random_offset;
+                                b = ts.realtime;
                         }
 
+                        /* We always subtract random_offset from the base time because
+                         * calendar_spec_next_usec() finds the next calendar event, and then we add the
+                         * offset back afterwards (see below). The base time needs to be in "pre-offset"
+                         * space so the next calendar match is computed correctly. Without this, a timer
+                         * that fires late (e.g. persistent catch-up) would skip the next scheduled
+                         * activation. */
+                        b = usec_sub_unsigned(b, random_offset);
+
                         r = calendar_spec_next_usec(v->calendar_spec, b, &v->next_elapse);
                         if (r < 0)
                                 continue;