core: Detect initial timer state from serialized data
authorMichal Koutný <mkoutny@suse.com>
Fri, 2 Nov 2018 19:56:08 +0000 (20:56 +0100)
committerLennart Poettering <lennart@poettering.net>
Wed, 21 Nov 2018 10:28:33 +0000 (11:28 +0100)
We keep a mark whether a single-shot timer was triggered in the caller's
variable initial. When such a timer elapses while we are
serializing/deserializing the inner state, we consider the timer
incorrectly as elapsed and don't trigger it later.

This patch exploits last_trigger timestamp that we already serialize,
hence we can eliminate the argument initial completely.

A reproducer for OnBootSec= timers:
        cat >repro.c <<EOD
        /*
         * Compile: gcc repro.c -o repro
         * Run: ./repro
         */
        #include <errno.h>
        #include <fcntl.h>
        #include <stdio.h>
        #include <stdlib.h>
        #include <sys/stat.h>
        #include <sys/types.h>
        #include <time.h>
        #include <unistd.h>

        int main(int argc, char *argv[]) {
         char command[1024];
         int pause;

         struct timespec now;

         while (1) {
         usleep(rand() % 200000); // prevent periodic repeats
                clock_gettime(CLOCK_MONOTONIC, &now);
         printf("%i\n", now.tv_sec);

         system("rm -f $PWD/mark");
         snprintf(command, 1024, "systemd-run --user --on-boot=%i --timer-property=AccuracySec=100ms "
         "touch $PWD/mark", now.tv_sec + 1);
         system(command);
         system("systemctl --user list-timers");
         pause = (1000000000 - now.tv_nsec)/1000 - 70000; // fiddle to hit the middle of reloading
         usleep(pause > 0 ? pause : 0);
         system("systemctl --user daemon-reload");
         sync();
         sleep(2);
         if (open("./mark", 0) < 0)
         if (errno == ENOENT) {
         printf("mark file does not exist\n");
         break;
         }
         }

         return 0;
        }
        EOD

src/core/timer.c

index 4e7fade..1527aab 100644 (file)
@@ -263,7 +263,7 @@ static void timer_set_state(Timer *t, TimerState state) {
         unit_notify(UNIT(t), state_translation_table[old_state], state_translation_table[state], 0);
 }
 
-static void timer_enter_waiting(Timer *t, bool initial, bool time_change);
+static void timer_enter_waiting(Timer *t, bool time_change);
 
 static int timer_coldplug(Unit *u) {
         Timer *t = TIMER(u);
@@ -275,7 +275,7 @@ static int timer_coldplug(Unit *u) {
                 return 0;
 
         if (t->deserialized_state == TIMER_WAITING)
-                timer_enter_waiting(t, false, false);
+                timer_enter_waiting(t, false);
         else
                 timer_set_state(t, t->deserialized_state);
 
@@ -331,7 +331,7 @@ static void add_random(Timer *t, usec_t *v) {
         log_unit_debug(UNIT(t), "Adding %s random time.", format_timespan(s, sizeof(s), add, 0));
 }
 
-static void timer_enter_waiting(Timer *t, bool initial, bool time_change) {
+static void timer_enter_waiting(Timer *t, bool time_change) {
         bool found_monotonic = false, found_realtime = false;
         bool leave_around = false;
         triple_timestamp ts;
@@ -441,7 +441,8 @@ static void timer_enter_waiting(Timer *t, bool initial, bool time_change) {
 
                         v->next_elapse = usec_add(usec_shift_clock(base, CLOCK_MONOTONIC, TIMER_MONOTONIC_CLOCK(t)), v->value);
 
-                        if (!initial && !time_change &&
+                        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 */
@@ -639,7 +640,7 @@ static int timer_start(Unit *u) {
         }
 
         t->result = TIMER_SUCCESS;
-        timer_enter_waiting(t, true, false);
+        timer_enter_waiting(t, false);
         return 1;
 }
 
@@ -754,14 +755,14 @@ static void timer_trigger_notify(Unit *u, Unit *other) {
         case TIMER_ELAPSED:
 
                 /* Recalculate sleep time */
-                timer_enter_waiting(t, false, false);
+                timer_enter_waiting(t, false);
                 break;
 
         case TIMER_RUNNING:
 
                 if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
                         log_unit_debug(UNIT(t), "Got notified about unit deactivation.");
-                        timer_enter_waiting(t, false, false);
+                        timer_enter_waiting(t, false);
                 }
                 break;
 
@@ -803,7 +804,7 @@ static void timer_time_change(Unit *u) {
                 t->last_trigger.realtime = ts;
 
         log_unit_debug(u, "Time change, recalculating next elapse.");
-        timer_enter_waiting(t, false, true);
+        timer_enter_waiting(t, true);
 }
 
 static void timer_timezone_change(Unit *u) {
@@ -815,7 +816,7 @@ static void timer_timezone_change(Unit *u) {
                 return;
 
         log_unit_debug(u, "Timezone change, recalculating next elapse.");
-        timer_enter_waiting(t, false, false);
+        timer_enter_waiting(t, false);
 }
 
 static const char* const timer_base_table[_TIMER_BASE_MAX] = {