Skip to content

Commit

Permalink
Remove firer struct & fix ticker reset (#95)
Browse files Browse the repository at this point in the history
Remove firer interface & fixt ticker reset

It also fixes an issue where ticker resets were not storing the new ticker duration. This would manifest as a ticker whose time was reset, but only for 1 tick, after which it would revert to the original time.
  • Loading branch information
DPJacques authored Nov 29, 2024
1 parent 91d2c0a commit 6d8d032
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 60 deletions.
54 changes: 8 additions & 46 deletions clockwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ type expirer interface {
expire(now time.Time) (next *time.Duration)

// Get and set the expiration time.
expiry() time.Time
setExpiry(time.Time)
expiration() time.Time
setExpiration(time.Time)
}

// After mimics [time.After]; it waits for the given duration to elapse on the
Expand Down Expand Up @@ -153,17 +153,7 @@ func (fc *FakeClock) NewTicker(d time.Duration) Ticker {
if d <= 0 {
panic(errors.New("non-positive interval for NewTicker"))
}
var ft *fakeTicker
ft = &fakeTicker{
firer: newFirer(),
d: d,
reset: func(d time.Duration) {
fc.l.Lock()
defer fc.l.Unlock()
fc.setExpirer(ft, d)
},
stop: func() { fc.stop(ft) },
}
ft := newFakeTicker(fc, d)
fc.l.Lock()
defer fc.l.Unlock()
fc.setExpirer(ft, d)
Expand Down Expand Up @@ -192,7 +182,7 @@ func (fc *FakeClock) newTimer(d time.Duration, afterfunc func()) (*fakeTimer, ti
fc.l.Lock()
defer fc.l.Unlock()
fc.setExpirer(ft, d)
return ft, ft.expiry()
return ft, ft.expiration()
}

// newTimerAtTime is like newTimer, but uses a time instead of a duration.
Expand All @@ -218,12 +208,12 @@ func (fc *FakeClock) Advance(d time.Duration) {
//
// We don't iterate because the callback of the waiter might register a new
// waiter, so the list of waiters might change as we execute this.
for len(fc.waiters) > 0 && !end.Before(fc.waiters[0].expiry()) {
for len(fc.waiters) > 0 && !end.Before(fc.waiters[0].expiration()) {
w := fc.waiters[0]
fc.waiters = fc.waiters[1:]

// Use the waiter's expiration as the current time for this expiration.
now := w.expiry()
now := w.expiration()
fc.time = now
if d := w.expire(now); d != nil {
// Set the new expiration if needed.
Expand Down Expand Up @@ -311,10 +301,10 @@ func (fc *FakeClock) setExpirer(e expirer, d time.Duration) {
return
}
// Add the expirer to the set of waiters and notify any blockers.
e.setExpiry(fc.time.Add(d))
e.setExpiration(fc.time.Add(d))
fc.waiters = append(fc.waiters, e)
slices.SortFunc(fc.waiters, func(a, b expirer) int {
return a.expiry().Compare(b.expiry())
return a.expiration().Compare(b.expiration())
})

// Notify blockers of our new waiter.
Expand All @@ -327,31 +317,3 @@ func (fc *FakeClock) setExpirer(e expirer, d time.Duration) {
return false
})
}

// firer is used by fakeTimer and fakeTicker used to help implement expirer.
type firer struct {
// The channel associated with the firer, used to send expiration times.
c chan time.Time

// The time when the firer expires. Only meaningful if the firer is currently
// one of a fakeClock's waiters.
exp time.Time
}

func newFirer() firer {
return firer{c: make(chan time.Time, 1)}
}

func (f *firer) Chan() <-chan time.Time {
return f.c
}

// expiry implements expirer.
func (f *firer) expiry() time.Time {
return f.exp
}

// setExpiry implements expirer.
func (f *firer) setExpiry(t time.Time) {
f.exp = t
}
35 changes: 29 additions & 6 deletions ticker.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ func (r realTicker) Chan() <-chan time.Time {
}

type fakeTicker struct {
firer
// The channel associated with the firer, used to send expiration times.
c chan time.Time

// The time when the ticker expires. Only meaningful if the ticker is currently
// one of a FakeClock's waiters.
exp time.Time

// reset and stop provide the implementation of the respective exported
// functions.
Expand All @@ -30,13 +35,27 @@ type fakeTicker struct {
d time.Duration
}

func (f *fakeTicker) Reset(d time.Duration) {
f.reset(d)
func newFakeTicker(fc *FakeClock, d time.Duration) *fakeTicker {
var ft *fakeTicker
ft = &fakeTicker{
c: make(chan time.Time, 1),
d: d,
reset: func(d time.Duration) {
fc.l.Lock()
defer fc.l.Unlock()
ft.d = d
fc.setExpirer(ft, d)
},
stop: func() { fc.stop(ft) },
}
return ft
}

func (f *fakeTicker) Stop() {
f.stop()
}
func (f *fakeTicker) Chan() <-chan time.Time { return f.c }

func (f *fakeTicker) Reset(d time.Duration) { f.reset(d) }

func (f *fakeTicker) Stop() { f.stop() }

func (f *fakeTicker) expire(now time.Time) *time.Duration {
// Never block on expiration.
Expand All @@ -46,3 +65,7 @@ func (f *fakeTicker) expire(now time.Time) *time.Duration {
}
return &f.d
}

func (f *fakeTicker) expiration() time.Time { return f.exp }

func (f *fakeTicker) setExpiration(t time.Time) { f.exp = t }
23 changes: 15 additions & 8 deletions timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ func (r realTimer) Chan() <-chan time.Time {
}

type fakeTimer struct {
firer
// The channel associated with the firer, used to send expiration times.
c chan time.Time

// The time when the firer expires. Only meaningful if the firer is currently
// one of a FakeClock's waiters.
exp time.Time

// reset and stop provide the implementation of the respective exported
// functions.
Expand All @@ -33,7 +38,7 @@ type fakeTimer struct {
func newFakeTimer(fc *FakeClock, afterfunc func()) *fakeTimer {
var ft *fakeTimer
ft = &fakeTimer{
firer: newFirer(),
c: make(chan time.Time, 1),
reset: func(d time.Duration) bool {
fc.l.Lock()
defer fc.l.Unlock()
Expand All @@ -49,13 +54,11 @@ func newFakeTimer(fc *FakeClock, afterfunc func()) *fakeTimer {
return ft
}

func (f *fakeTimer) Reset(d time.Duration) bool {
return f.reset(d)
}
func (f *fakeTimer) Chan() <-chan time.Time { return f.c }

func (f *fakeTimer) Stop() bool {
return f.stop()
}
func (f *fakeTimer) Reset(d time.Duration) bool { return f.reset(d) }

func (f *fakeTimer) Stop() bool { return f.stop() }

func (f *fakeTimer) expire(now time.Time) *time.Duration {
if f.afterFunc != nil {
Expand All @@ -70,3 +73,7 @@ func (f *fakeTimer) expire(now time.Time) *time.Duration {
}
return nil
}

func (f *fakeTimer) expiration() time.Time { return f.exp }

func (f *fakeTimer) setExpiration(t time.Time) { f.exp = t }

0 comments on commit 6d8d032

Please sign in to comment.