commit 77cc97cf0a20ed0a062a1cb87bef6c40941e4cff Author: David Goulet dgoulet@torproject.org Date: Mon Sep 18 10:47:05 2017 -0400
sched: Don't cast to int32_t the monotime_diff_msec() result
When the KIST schedule() is called, it computes a diff value between the last scheduler run and the current monotonic time. If tha value is below the run interval, the libevent even is updated else the event is run.
It turned out that casting to int32_t the returned int64_t value for the very first scheduler run (which is set to 0) was creating an overflow on the 32 bit value leading to adding the event with a gigantic usec value. The scheduler was simply never running for a while.
First of all, a BUG() is added for a diff value < 0 because if the time is really monotonic, we should never have a now time that is lower than the last scheduler run time. And we will try to recover with a diff value to 0.
Second, the diff value is changed to int64_t so we avoid this "bootstrap overflow" and future casting overflow problems.
Fixes #23558
Signed-off-by: David Goulet dgoulet@torproject.org --- src/or/scheduler_kist.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index ba8d416db..e4ae07107 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -507,16 +507,25 @@ kist_scheduler_schedule(void) { struct monotime_t now; struct timeval next_run; - int32_t diff; + int64_t diff;
if (!have_work()) { return; } monotime_get(&now); - diff = (int32_t) monotime_diff_msec(&scheduler_last_run, &now); + + /* If time is really monotonic, we can never have now being smaller than the + * last scheduler run. The scheduler_last_run at first is set to 0. */ + diff = monotime_diff_msec(&scheduler_last_run, &now); + IF_BUG_ONCE(diff < 0) { + diff = 0; + } if (diff < sched_run_interval) { next_run.tv_sec = 0; - /* 1000 for ms -> us */ + /* Takes 1000 ms -> us. This will always be valid because diff can NOT be + * negative and can NOT be smaller than sched_run_interval so values can + * only go from 1000 usec (diff set to interval - 1) to 100000 usec (diff + * set to 0) for the maximum allowed run interval (100ms). */ next_run.tv_usec = (sched_run_interval - diff) * 1000; /* Readding an event reschedules it. It does not duplicate it. */ scheduler_ev_add(&next_run);