KEMBAR78
time: Timer.Reset is not possible to use correctly · Issue #14038 · golang/go · GitHub
Skip to content

time: Timer.Reset is not possible to use correctly #14038

@dvyukov

Description

@dvyukov

I saw two common usage patterns of Timer.Reset:

First is ensuring that a sequence of events happens in a timely fashion:

timeout := time.NewTimer(T)
for {
  if !timeout.Reset(T) {
    <-timeout.C
  }
  select {
  case <- ...: ...
  case <-timeout.C: ...
  }
}

Second is connection timeout with an ability to change the timeout:

timeout := time.NewTimer(T)

// goroutine 1
for {
  select {
  case <- ...: ...
  case <-timeout.C: ...
  }
}

// goroutine 2
  if !timeout.Reset(T2) {
    <-timeout.C
  }

There are several things that can go wrong here depending on timings:

  1. Reset returns false; one element is already in the channel (the old one); timer goroutine sends another (corresponding to the new timeout), it is discarded since the channel is full; now we drain the channel to read out the old value; as the result timeout will never happen as the new value was discarded.
  2. Reset returns false; one element is already in the channel (the old one); timer goroutine sends another (corresponding to the new timeout), it is discarded since the channel is full; goroutine 1 reads out from the channel; goroutine 2 hangs forever trying to drain the channel.

I've found several cases of this bug in our internal codebase, one even with a TODO by @bradfitz describing this race.

Timer.Reset does the following:

func (t *Timer) Reset(d Duration) bool {
    if t.r.f == nil {
        panic("time: Reset called on uninitialized Timer")
    }
    w := when(d)
    active := stopTimer(&t.r)
    t.r.when = w
    startTimer(&t.r)
    return active
}

What would be possible to use correctly is:

func (t *Timer) Reset2(d Duration) bool {
    if t.r.f == nil {
        panic("time: Reset called on uninitialized Timer")
    }
    w := when(d)
    active := stopTimer(&t.r)
        if !active {
                <-t.C
        }
    t.r.when = w
    startTimer(&t.r)
    return active
}

@rsc @Sajmani @aclements

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions