Opened 3 months ago

Closed 5 weeks ago

#29640 closed defect (fixed)

Improve the monotonic time documentation in compat_time.c

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version: Tor: 0.2.9.1-alpha
Severity: Normal Keywords: tor-time, asn-merge
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: nickm Sponsor:

Description

We discovered in #29500 that the monotonic time documentation is incomplete. Here are some improvements we could make:

  • define "monotonic" as "strictly non-decreasing, delta t >= 0"
  • explain when 0 time deltas are more likely:
    • when measuring units larger than nanoseconds (due to truncation on division)
    • on platforms with low resolution timers (because two time calls can happen faster than the resolution of the timer)
    • on Windows, where the time is not always monotonic, due to an OS bug
    • on platforms without specialised monotonic functions, during a wall clock time change
  • explain what happens when the timers overflow?

Based on:
https://gitweb.torproject.org/tor.git/tree/src/lib/time/compat_time.c#n175
https://trac.torproject.org/projects/tor/ticket/29500#comment:11

Child Tickets

Change History (10)

comment:1 Changed 3 months ago by teor

Actual Points: 0.1
Keywords: 040-backport added
Status: assignedneeds_review
Version: Tor: 0.2.9.1-alpha

See my pull request on 0.4.0:
https://github.com/torproject/tor/pull/754

Since 0.4.0 is in alpha, we might want to backport documentation fixes.

comment:2 Changed 3 months ago by nickm

  • on Windows, where the time is not always monotonic, due to an OS bug
  • on platforms without specialised monotonic functions, during a wall clock time change

FWIW, I think our code does some ratcheting to ensure that even if we're basing our code on non-monotonic timers, our outputs never move backwards.

comment:3 Changed 3 months ago by dgoulet

Reviewer: mikeperry

comment:4 in reply to:  2 Changed 3 months ago by teor

Replying to nickm:

  • on Windows, where the time is not always monotonic, due to an OS bug
  • on platforms without specialised monotonic functions, during a wall clock time change

FWIW, I think our code does some ratcheting to ensure that even if we're basing our code on non-monotonic timers, our outputs never move backwards.

Yes, the code does some ratcheting to make sure that outputs never move backwards. But that's not the same as always moving forwards, because the result can be zero.

I separated the ratchet and non-ratchet cases in a fixup, and explained them better.

comment:5 Changed 7 weeks ago by teor

Reviewer: mikeperry

Remove Mike as reviewer, because he's overloaded.
We'll work out what to do with these tickets in the weekly meeting.

comment:6 Changed 6 weeks ago by asn

Reviewer: nickm

comment:7 Changed 6 weeks ago by nickm

Status: needs_reviewmerge_ready

LGTM; I don't think we need to backport though.

comment:8 Changed 6 weeks ago by nickm

Keywords: asn-merge added

comment:9 Changed 6 weeks ago by teor

Keywords: 040-backport removed

comment:10 Changed 5 weeks ago by asn

Resolution: fixed
Status: merge_readyclosed

merged to master. did not backport as indicated by comment:7.

Note: See TracTickets for help on using tickets.