Opened 4 months ago

Last modified 4 months ago

#25113 merge_ready defect

monotonic_time unit test fail, 0.3.3.1-alpha debian armel

Reported by: arma Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version: Tor: 0.3.3.1-alpha
Severity: Normal Keywords: tor-test, 029-backport, 031-backport, 032-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

<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. :)

Child Tickets

Change History (5)

comment:1 Changed 4 months ago by dgoulet

Keywords: tor-test added
Status: newneeds_information

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.

comment:2 Changed 4 months ago by dgoulet

Owner: set to dgoulet
Status: needs_informationaccepted

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

comment:3 Changed 4 months ago by dgoulet

Status: acceptedmerge_ready

Kind of trivial fix so merge_ready.

comment:4 Changed 4 months ago by dgoulet

Keywords: 029-backport 031-backport 032-backport added

Oh and the 029 branch for backport: bug25113_029_01

comment:5 Changed 4 months ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: 0.3.2.x-final

Merged to master; marking as backportable.

Note: See TracTickets for help on using tickets.