#5797·go-zero

Bug: TimingWheel.MoveTimer can fire one revolution early or late

Author: purplecityCreated Sep 26, 2026Updated Sep 26, 2026

Description

TimingWheel.MoveTimer (and SetTimer on an existing key) can fire a task a full revolution early or late when the move is issued after the wheel has been running — i.e. whenever the task's old slot differs from the current pointer position.

Environment

  • OS: macOS 26.7 (arm64) — the reproduction below drives time manually via FakeTicker, so it is platform/time-independent
  • Go: go1.27.1
  • go-zero: master @ 84c92d71

Minimal reproduction

go
func TestMoveMidFlight(t *testing.T) {
	// 6-slot wheel; SetTimer 20s at t=0, MoveTimer 14s right after tick 3
	ticker := timex.NewFakeTicker()
	var tickNow, fireTick int32 = 0, -1
	tw, _ := collection.NewTimingWheelWithTicker(time.Second, 6, func(k, v any) {
		atomic.StoreInt32(&fireTick, atomic.LoadInt32(&tickNow))
	}, ticker)
	defer tw.Stop()

	tw.SetTimer("A", 1, 20*time.Second)
	for i := 1; i <= 30; i++ {
		atomic.StoreInt32(&tickNow, int32(i))
		ticker.Tick()
		time.Sleep(3 * time.Millisecond)
		if i == 3 {
			tw.MoveTimer("A", 14*time.Second)
			time.Sleep(3 * time.Millisecond)
		}
	}
	t.Log("fired at:", atomic.LoadInt32(&fireTick))
}

Intended fire: t=17 (move at t=3 + 14s). Actual: t=23, six ticks (one revolution) late. The mirror geometry (task slot ahead of the pointer, multi-revolution move) fires one revolution early: Set 18s + Move 23s at t=4 on a 12-slot wheel → intended t=27, actual t=15.

Root cause

moveTask derives pos/circle from getPositionAndCircle, whose (steps-1)/N circle formula is written for a fresh entry hung directly at the target slot. But a moved task physically stays in its old slot Q; the circle countdown and the diff hop only take effect when the pointer scans Q, which is

a = ((Q - tickedPos - 1) mod N) + 1     // 1..N ticks away

This arc a is never credited against the delay budget, so the actual fire time is a + circle*N + diff while the intended one is steps:

fire - steps = (Q-P-1 mod N) + pos - Q - ((steps-1) mod N) ≡ 0 (mod N)

The written diff is always correct (≡ steps - a, mod N), so the entire error lands on circle: it is exactly 0 or ±N.

  • old slot already passed this revolution, target ahead (pos >= timer.pos branch): fires N late
  • old slot not yet passed, target behind & multi-revolution (pos < timer.pos && circle > 0 branch): fires N early

Why existing tests don't catch it

All current Move tests issue SetTimer and MoveTimer at the same instant (same anchor position), a geometry where the arithmetic happens to be exact.

Impact

core/collection/cache.go calls MoveTimer on every Set of an existing key (wheel: 1s x 300 slots, one revolution = 5 minutes), so in-memory cache expiry can be off by up to ±5 minutes. Tasks are never lost or duplicated, only mistimed. stores/cache/cleaner.go is unaffected (it re-SetTimers a fresh key after the previous entry fired).

Suggested fix

Credit the arc a before converting the remaining budget:

go
steps := int(task.delay / tw.interval)
// ticks until the pointer next scans the old slot, in [1, numSlots]
var a int
if tw.tickedPos > timer.pos {
	a = tw.numSlots - (tw.tickedPos - timer.pos) // old slot behind: wrap around
} else if tw.tickedPos == timer.pos {
	a = tw.numSlots // current slot was just scanned: wait a full revolution
} else {
	a = timer.pos - tw.tickedPos // old slot ahead: walk the difference
}
// the three branches above are equivalent to the one-liner:
// a = (timer.pos-tw.tickedPos-1+tw.numSlots)%tw.numSlots + 1

if steps < a {
	// intended fire precedes the next scan of the old slot: re-hang immediately
	timer.item.removed = true
	newItem := &timingEntry{baseEntry: task, value: timer.item.value}
	pos, circle := tw.getPositionAndCircle(task.delay)
	newItem.circle = circle
	tw.slots[pos].PushBack(newItem)
	tw.setTimerPosition(pos, newItem)
	return
}
timer.item.circle = (steps - a) / tw.numSlots
timer.item.diff = (steps - a) % tw.numSlots

All existing tests pass unchanged with this fix; I also have table-driven regression tests for mid-flight moves (the geometry above) ready to PR if welcome.