From 6d8d032a18422c2e3ef651170a8a55012d1f704c Mon Sep 17 00:00:00 2001 From: Darren Jacques Date: Fri, 29 Nov 2024 10:02:53 -0800 Subject: [PATCH] Remove firer struct & fix ticker reset (#95) 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. --- clockwork.go | 54 ++++++++-------------------------------------------- ticker.go | 35 ++++++++++++++++++++++++++++------ timer.go | 23 ++++++++++++++-------- 3 files changed, 52 insertions(+), 60 deletions(-) diff --git a/clockwork.go b/clockwork.go index b987f66..85a9934 100644 --- a/clockwork.go +++ b/clockwork.go @@ -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 @@ -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) @@ -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. @@ -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. @@ -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. @@ -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 -} diff --git a/ticker.go b/ticker.go index b68e4d7..aa56952 100644 --- a/ticker.go +++ b/ticker.go @@ -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. @@ -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. @@ -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 } diff --git a/timer.go b/timer.go index e45c25f..e7e1d40 100644 --- a/timer.go +++ b/timer.go @@ -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. @@ -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() @@ -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 { @@ -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 }