time: Timer.Reset is not possible to use correctly (original) (raw)

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