tor_gettimeofday_cached_monotonic, as designed, could stall indefinitely

As designed, our implementation of faked monotonic time can pause indefinitely if clock time jumps backwards. That probably isn't such a good idea.

(See #18906 (moved) for why I'm saying "as designed" here)

changed milestone to %Tor: 0.2.9.x-final

added actualpoints::0 component::core tor/tor milestone::Tor: 0.2.9.x-final points::1 priority::medium resolution::fixed severity::normal status::closed type::defect labels

I think the code for a 'fixed' version would look something like this. (Untested, probably has a sign bug or something.)

void
tor_gettimeofday_cached_monotonic(struct timeval *tv)
{
  static struct timeval last_tv = { 0, 0 };
  static struct timeval offset = { 0, 0 };

  tor_gettimeofday_cached(tv);
  timeradd(tv, &offset, tv);

  if (timercmp(tv, &last_tv, OP_LT)) {
    /* Time jumped backwards.  Instead, we're going to increment 'offset' by
       enough so that no time passed at all. */
    timersub(&last_tv, tv, &offset);
    memcpy(tv, &last_tv, sizeof(struct timeval));
  } else {
    memcpy(&last_tv, tv, sizeof(struct timeval));
  }
}

Trac:
Points: N/A to small

marking these as the tickets I am in favor of for 029, among the 029-proposed ones.

Trac:
Keywords: N/A deleted, 029-nickm-says-yes added

Nobody objected to including any of these, so I guess they are in.

Trac:
Milestone: N/A to Tor: 0.2.9.x-final
Keywords: 029-proposed, 029-nickm-says-yes deleted, N/A added

As I said last week on IRC, I did question these. I'm not sure that this actually solves the problem of stalling. Based on running through this by hand on paper, it seems like we will just be maintaining the offset permanently as the delta of the backwards clock jump. This means that the function will continue to return times at least offset seconds in the future from the current clock. This means that when timeouts are scheduled by #18365 (moved), they will always be offset seconds late when they fire (since they are scheduled relative to this function and not the current time).

Nick, last week, you agreed to test that. Did you? If not, I don't think this is in. And I still object as I did before until that point.

As I said last week on IRC, I did question these.

Sorry, I meant "Nobody objected to including any of these [tickets in 0.2.9]".

Trac:
Points: small to 1

tor_gettimeofday_cached_monotonic is now dead; see #18908 (moved).

Trac:
Resolution: N/A to fixed
Status: new to closed
Actualpoints: N/A to 0

closed

changed time estimate to 8h

moved to tpo/core/tor#18907 (closed)