Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#18908 closed enhancement (fixed)

Implement monotonic time in a better way than simply calling down to libevent's non-monotonic time

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-5 TorCoreTeam201607
Cc: Actual Points: 1
Parent ID: Points: 1
Reviewer: athena Sponsor:


For a synopsis of the various better ways, see what libevent 2.1 will someday do, in .

Child Tickets

Change History (14)

comment:1 Changed 5 years ago by nickm

Points: small

comment:2 Changed 5 years ago by nickm

Keywords: 029-nickm-unsure added

Marking these tickets as the ones I think I need more feedback about in order to figure out if I think it should go in 0.2.9.

comment:3 Changed 5 years ago by nickm

Milestone: Tor: 0.2.9.x-final

Calling these "yes for 0.2.9" since a good implementation of mike's padding patches requires them.

comment:4 Changed 5 years ago by nickm

Keywords: 029-proposed. 029-nickm-unsure removed

comment:5 Changed 4 years ago by nickm

Points: small1

comment:6 Changed 4 years ago by nickm

Owner: set to nickm
Status: newaccepted

I've started this in a branch monotonic_v2. More work is needed: I need to test it and finish the windows code.

comment:7 Changed 4 years ago by nickm

Keywords: review-group-5 added

comment:8 Changed 4 years ago by nickm

Reviewer: athena

comment:9 Changed 4 years ago by nickm

Actual Points: 1
Status: acceptedneeds_review

Now with unit tests and a changes file

comment:10 Changed 4 years ago by andrea

Begin code review for nickm/monotonic_v2:

Summary: I think I'm good with this being merged as long as all four cases
(clock_gettime(), gettimeofday(), Windows and Mac OS) got tested.


  • Just moving code here. Looks good to me.


  • Good, this doesn't make any particular assumptions about the origin of monotonic time. On current Linux, CLOCK_MONOTONIC is actually since system boot. The parts for platforms I know look okay to me, but did we manage to actually test the Windows / Mac OS X stuff?

(cf. e431ed37360a405f36e07a8c902146f0b4f4b31b)


  • Looks fine.


  • Looks okay.


  • Hmm, looks okay but have we tested it on Windows yet?


  • Just test suite exposure.


  • Okay, it compiles but have we tested it? :P


  • Woot, a pure code deletion commit


  • Looks fine.


  • Unit tests only in this one; they look fine.


  • Just the changes file

End code review

comment:11 Changed 4 years ago by nickm

Status: needs_reviewmerge_ready

Your main questions were about testing. I ran the tests on Linux, OSX, and on Linux with clock_gettime artificially disabled. They all seemed to work okay. I compiled the windows tests, but didn't have an easy way to run them. I think I'll just merge, though, and see what Jenkins thinks? Unless you object.

comment:12 Changed 4 years ago by andrea

Yeah, I think I'm happy with that.

comment:13 Changed 4 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

Okay. Squashed and merged.

comment:14 Changed 4 years ago by nickm

Keywords: TorCoreTeam201607 added
Note: See TracTickets for help on using tickets.