monotonic_time unit test fail, 0.3.3.1-alpha debian armel

[arma]

<weasel> util/monotonic_time:
<weasel>   FAIL ../src/test/test_util.c:5843: assert(msecc1 OP_LE nsecc1 / 1000000 + 1): 41399 vs 41395
<weasel>   [monotonic_time FAILED]
<weasel> https://buildd.debian.org/status/fetch.php?pkg=tor&arch=armel&ver=0.3.3.1-alpha-1&stamp=1517099318&raw=0

Looks like the armel build failed:
​https://buildd.debian.org/status/package.php?p=tor&suite=experimental

The comment in the unit tests right above that is

  /* We need to be a little careful here since we don't know the system load.

which makes me suspicious. :)

[dgoulet]

Wow ok so there is literally a 4msec gap between the nsecc1 and msecc1 function call on armel.

  nsecc1 = monotime_coarse_absolute_nsec();
  [usecc1 = monotime_coarse_absolute_usec();]
  msecc1 = monotime_coarse_absolute_msec();

So this test is doomed to fail on slow system because we get nsecc1 before msecc1 so shouldn't be msecc1 >= nsecc1 instead?

  tt_u64_op(msecc1, OP_LE, nsecc1 / 1000000 + 1);
  -> msecc1 <= nsecc1

Actually the whole set of checks between the 1 variables seems a bit weird, it assumes the clock gettime calls to be in the same msec range.

Also, clock_gettime() is a vdso in ARM exported since Linux 4.1 so if the build machine has an older kernel, the time gaps can be bigger.

[dgoulet]

Quick discussion with nickm on IRC, bumping to 10msec is most likely what we want because we need to check for the synchronization between time read. Not ideal fix but for now that should do it.

If we ever encounter a system that will have a 10msec+ gap between the function call, we'll adapt at that point.

Branch: bug25113_033_01

[dgoulet]

Kind of trivial fix so merge_ready.

[dgoulet]

Oh and the 029 branch for backport: bug25113_029_01

[nickm]

Merged to master; marking as backportable.