Description
The following unit test fails with a panic:
func TestQuartz(t *testing.T) {
clock := quartz.NewMock(t)
doneChan := make(chan any)
go func() {
timer := clock.NewTimer(0 * time.Second)
<- timer.C
close(doneChan)
}()
newTimerTrap := clock.Trap().NewTimer()
defer newTimerTrap.Close()
clock.AdvanceNext()
<- doneChan
log.Infof("Done at %s", clock.Now().String())
}The issue is that I'm setting a timer with duration 0 which is legal. However, if you look at AdvanceNext you see:
func (m *Mock) AdvanceNext() (time.Duration, AdvanceWaiter) {
m.mu.Lock()
m.tb.Helper()
w := AdvanceWaiter{tb: m.tb, ch: make(chan struct{})}
if m.nextTime.IsZero() {
defer close(w.ch)
defer m.mu.Unlock()
m.tb.Error("cannot AdvanceNext because there are no timers or tickers running")
}
d := m.nextTime.Sub(m.cur)
m.cur = m.nextTime
go m.advanceLocked(w)
return d, w
}There's two bugs here I think:
- It checks if
m.nextTime.IsZerobut that doesn't necessarily mean there's nothing scheduled as it is legal to set a timer with a duration of 0 - If it does hit that condition it calls
defer m.mu.Unlockbut then it still callsadvanceLockedwhich unlocks that mutex so you get a panic due to a double unlock of the mutex.
Granted it's a bit of an odd case to schedule a timer for 0 seconds in the future but I think there are valid use cases for it. For my use case a user puts a request for a function to be called into a channel. We read from that channel and schedule the callback. It is possible that by the time we read from the channel the time has expired so we should call right away. We could special case that code but as it's legal to schedule a timer with 0-delay with a real timer and that's less code and fewere if statements we simply schedule the timer to fire immediately.