Skip to content

Commit f2958d3

Browse files
committed
Fix tick/reset/stop race
We release the lock before ticking which means the timer could have been reset. This can lead to the following issue: 1. Something triggers a timer, calling `Tick`. 2. Concurrently, `Stop` is called. `Stop` returns false because the timer was running and we succeed in stopping the timer. 3. `Tick` takes the lock and triggers the timer anyways. The fix is to check if we're stopped inside the timer. Maybe we should avoid dropping the lock in the first place but I want to focus on the direct fix for now.
1 parent da6a910 commit f2958d3

File tree

1 file changed

+21
-6
lines changed

1 file changed

+21
-6
lines changed

clock.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,10 @@ func (t *Timer) Reset(d time.Duration) bool {
325325
}
326326

327327
t.mock.mu.Lock()
328-
t.next = t.mock.now.Add(d)
329328
defer t.mock.mu.Unlock()
330329

330+
t.next = t.mock.now.Add(d)
331+
331332
registered := !t.stopped
332333
if t.stopped {
333334
t.mock.timers = append(t.mock.timers, (*internalTimer)(t))
@@ -346,6 +347,13 @@ func (t *internalTimer) Tick(now time.Time) {
346347
defer gosched()
347348

348349
t.mock.mu.Lock()
350+
defer t.mock.mu.Unlock()
351+
352+
// Check if we're stopped, we could have been de-registered after taking the lock and before
353+
// calling tick.
354+
if t.stopped {
355+
return
356+
}
349357
if t.fn != nil {
350358
// defer function execution until the lock is released, and
351359
defer func() { go t.fn() }()
@@ -357,9 +365,8 @@ func (t *internalTimer) Tick(now time.Time) {
357365
default:
358366
}
359367
}
360-
t.mock.removeClockTimer((*internalTimer)(t))
361368
t.stopped = true
362-
t.mock.mu.Unlock()
369+
t.mock.removeClockTimer((*internalTimer)(t))
363370
}
364371

365372
// Ticker holds a channel that receives "ticks" at regular intervals.
@@ -408,14 +415,22 @@ type internalTicker Ticker
408415

409416
func (t *internalTicker) Next() time.Time { return t.next }
410417
func (t *internalTicker) Tick(now time.Time) {
418+
defer gosched()
419+
420+
t.mock.mu.Lock()
421+
defer t.mock.mu.Unlock()
422+
423+
// Check if we're stopped, we could have been de-registered after taking the lock and before
424+
// calling tick.
425+
if t.stopped {
426+
return
427+
}
428+
411429
select {
412430
case t.c <- now:
413431
default:
414432
}
415-
t.mock.mu.Lock()
416433
t.next = now.Add(t.d)
417-
t.mock.mu.Unlock()
418-
gosched()
419434
}
420435

421436
// Sleep momentarily so that other goroutines can process.

0 commit comments

Comments
 (0)