BUG/MINOR: time/threads: ensure the adjusted time is always correct

In the time offset calculation loop, we ensure we only commit the new
date once it's futher in the future than the current one. However there
is a small issue here on 32-bit platforms : if global_now is written in
two cycles by another thread, starting with the tv_sec part, and the
current thread reads it in the middle of a change, it may compute a
wrong "adjusted" value on the first round, with the new (larger) tv_sec
and the old (large) tv_usec. This will be detected as the CAS will fail,
and another attempt will be made, but this time possibly with too large
an adusted value, pushing the date further than needed (at worst almost
one second).

This patch addresses this by using a temporary adjusted time in the loop
that always restarts from the last known one, and by assigning the result
to the final value only once the CAS succeeds.

The impact is very limited, it may cause the time to advance in small
jumps on 32 bit platforms and in the worst case some timeouts might
expire 1 second too early.

This fix should be backported to 1.8.

diff --git a/src/time.c b/src/time.c
index 789b47c1ab079f21cdb56eb5df523e2b50fdd965..0de69627ccfb81f551cf51bf2d807cdb2ab752bf 100644 (file)
--- a/src/time.c
+++ b/src/time.c
@@ -173,7 +173,7 @@ REGPRM2 int _tv_isgt(const struct timeval *tv1, const struct timeval *tv2)
  */
 REGPRM2 void tv_update_date(int max_wait, int interrupted)
 {
-       struct timeval adjusted, deadline, tmp_now;
+       struct timeval adjusted, deadline, tmp_now, tmp_adj;
        unsigned int   curr_sec_ms;     /* millisecond of current second (0..999) */
        unsigned long long old_now;
        unsigned long long new_now;
@@ -215,18 +215,21 @@ REGPRM2 void tv_update_date(int max_wait, int interrupted)
        do {
                tmp_now.tv_sec  = (unsigned int)(old_now >> 32);
                tmp_now.tv_usec = old_now & 0xFFFFFFFFU;
+               tmp_adj = adjusted;
 
-               if (__tv_islt(&adjusted, &tmp_now))
-                       adjusted = tmp_now;
+               if (__tv_islt(&tmp_adj, &tmp_now))
+                       tmp_adj = tmp_now;
 
                /* now <adjusted> is expected to be the most accurate date,
                 * equal to <global_now> or newer.
                 */
-               new_now = (((unsigned long long)adjusted.tv_sec) << 32) + (unsigned int)adjusted.tv_usec;
+               new_now = (((unsigned long long)tmp_adj.tv_sec) << 32) + (unsigned int)tmp_adj.tv_usec;
 
                /* let's try to update the global <now> or loop again */
        } while (!HA_ATOMIC_CAS(&global_now, &old_now, new_now));
 
+       adjusted = tmp_adj;
+
        /* the new global date when we looked was old_now, and the new one is
         * new_now == adjusted. We can recompute our local offset.
         */