ipcpd, lib: Fix timerwheel issues / time_utils

Timerwheel would skip some additions due to a missed wakeup signal.

Addition of timespecs and timevals in the time utilities would
overflow tv_nsec/tv_usec if the sum is an integer number of seconds.

3 files changed, 31 insertions, 21 deletions

diff --git a/src/ipcpd/tests/timerwheel_test.c b/src/ipcpd/tests/timerwheel_test.c
index 615e6e41..9274577a 100644
--- a/src/ipcpd/tests/timerwheel_test.c
+++ b/src/ipcpd/tests/timerwheel_test.c
@@ -54,16 +54,18 @@ int timerwheel_test(int argc, char ** argv)
         (void) argc;
         (void) argv;
 
-        srand(time(NULL));
-
         total = 0;
 
+        srand(time(NULL));
+
         resolution = rand() % (MAX_RESOLUTION - 1) + 1;
         elements = rand() % (MAX_ELEMENTS - 10) + 10;
 
         tw = timerwheel_create(resolution, resolution * elements);
-        if (tw == NULL)
+        if (tw == NULL) {
+                printf("Failed to create timerwheel.\n");
                 return -1;
+        }
 
         wait.tv_sec = (resolution * elements) / 1000;
         wait.tv_nsec = ((resolution * elements) % 1000) * MILLION;
@@ -74,19 +76,24 @@ int timerwheel_test(int argc, char ** argv)
                 int delay = rand() % (resolution * elements);
                 int var = rand() % 5;
                 check_total += var;
-                timerwheel_add(tw,
-                               (void (*)(void *)) add,
-                               (void *) &var,
-                               sizeof(var),
-                               delay);
+                if (timerwheel_add(tw,
+                                   (void (*)(void *)) add,
+                                   (void *) &var,
+                                   sizeof(var),
+                                   delay)) {
+                        printf("Failed to add function.");
+                        return -1;
+                }
         }
 
         nanosleep(&wait, NULL);
 
         timerwheel_destroy(tw);
 
-        if (total != check_total)
+        if (total != check_total) {
+                printf("Totals do not match.\n");
                 return -1;
+        }
 
         return 0;
 }
diff --git a/src/ipcpd/timerwheel.c b/src/ipcpd/timerwheel.c
index f6785611..042f9cb3 100644
--- a/src/ipcpd/timerwheel.c
+++ b/src/ipcpd/timerwheel.c
@@ -136,10 +136,8 @@ static void * worker(void * o)
 
         while (tw_get_state(tw) == TW_RUNNING) {
                 if (pthread_cond_timedwait(&tw->work, &tw->lock, &dl)
-                    == ETIMEDOUT) {
+                    == ETIMEDOUT)
                         ts_add(&dl, &tw->intv, &dl);
-                        continue;
-                }
 
                 list_for_each_safe(p, h, &tw->wq) {
                         struct tw_f * f = list_entry(p, struct tw_f, next);
@@ -149,6 +147,7 @@ static void * worker(void * o)
                         if (f->arg != NULL)
                                 free(f->arg);
                         free(f);
+
                         pthread_mutex_lock(&tw->lock);
                 }
         }
@@ -162,7 +161,7 @@ static void * movement(void * o)
 {
         struct timerwheel * tw = (struct timerwheel *) o;
         struct timespec now = {0, 0};
-        long ms = (tw->resolution * tw->elements);
+        long ms = tw->resolution * tw->elements;
         struct timespec total = {ms / 1000,
                                  (ms % 1000) * MILLION};
         struct list_head * p;
@@ -170,18 +169,19 @@ static void * movement(void * o)
 
         while (tw_get_state(tw) == TW_RUNNING) {
                 clock_gettime(CLOCK_MONOTONIC, &now);
+
+                pthread_mutex_lock(&tw->lock);
+
                 if (ts_diff_us(&tw->wheel[tw->pos].expiry, &now) < 0) {
+                        pthread_mutex_unlock(&tw->lock);
                         nanosleep(&tw->intv, NULL);
                         continue;
                 }
 
-                pthread_mutex_lock(&tw->lock);
-
                 list_for_each_safe(p, h, &tw->wheel[tw->pos].funcs) {
                         struct tw_f * f = list_entry(p, struct tw_f, next);
                         list_del(&f->next);
                         list_add(&f->next, &tw->wq);
-                        pthread_cond_signal(&tw->work);
                 }
 
                 ts_add(&tw->wheel[tw->pos].expiry,
@@ -190,6 +190,8 @@ static void * movement(void * o)
 
                 tw->pos = (tw->pos + 1) & (tw->elements - 1);
 
+                pthread_cond_signal(&tw->work);
+
                 pthread_mutex_unlock(&tw->lock);
         }
 
@@ -228,8 +230,8 @@ struct timerwheel * timerwheel_create(unsigned int resolution,
 
         tw->resolution = resolution;
 
-        tw->intv.tv_sec = tw->resolution / (1000 * FRAC);
-        tw->intv.tv_nsec = (tw->resolution % 1000) * (MILLION / FRAC);
+        tw->intv.tv_sec = (tw->resolution / FRAC) / 1000;
+        tw->intv.tv_nsec = ((tw->resolution / FRAC) % 1000) * MILLION;
 
         INIT_LIST_HEAD(&tw->wq);
 
@@ -335,7 +337,7 @@ int timerwheel_add(struct timerwheel * tw,
                    size_t arg_len,
                    unsigned int delay)
 {
-        int pos = (tw->pos + delay / tw->resolution) & (tw->elements - 1);
+        int pos;
         struct tw_f * f = malloc(sizeof(*f));
         if (f == NULL)
                 return -ENOMEM;
@@ -353,6 +355,7 @@ int timerwheel_add(struct timerwheel * tw,
 
         pthread_mutex_lock(&tw->lock);
 
+        pos = (tw->pos + delay / tw->resolution) & (tw->elements - 1);
         list_add(&f->next, &tw->wheel[pos].funcs);
 
         pthread_mutex_unlock(&tw->lock);
diff --git a/src/lib/time_utils.c b/src/lib/time_utils.c
index b34e4763..4da935c4 100644
--- a/src/lib/time_utils.c
+++ b/src/lib/time_utils.c
@@ -39,7 +39,7 @@ int ts_add(const struct timespec * t,
         nanos = t->tv_nsec + intv->tv_nsec;
 
         res->tv_sec = t->tv_sec + intv->tv_sec;
-        while (nanos > BILLION) {
+        while (nanos >= BILLION) {
                 nanos -= BILLION;
                 ++(res->tv_sec);
         }
@@ -85,7 +85,7 @@ int tv_add(const struct timeval * t,
         micros = t->tv_usec + intv->tv_usec;
 
         res->tv_sec = t->tv_sec + intv->tv_sec;
-        while (micros > MILLION) {
+        while (micros >= MILLION) {
                 micros -= MILLION;
                 --(res->tv_sec);
         }