test: stabilize chatdebug heartbeat threshold test (#25022)

`launchHeartbeat` could miss a stale-threshold update during startup if
`SetStaleAfter` ran after the heartbeat ticker was created but before
the goroutine subscribed to `thresholdChan`. In that case, the heartbeat
kept the old interval until a future tick, and the mock-clock test could
time out waiting for `Ticker.Reset` without advancing time.

Subscribe to `thresholdChan` before reading the heartbeat interval so
the channel consistently invalidates the interval. The regression test
now changes the threshold while ticker creation is trapped, making the
startup race deterministic.

Closes https://github.com/coder/internal/issues/1513
This commit is contained in:
Ethan
2026-05-07 17:12:14 +10:00
committed by GitHub
parent 100ebd9f3b
commit 2ff05608d2
2 changed files with 20 additions and 14 deletions
+13 -9
View File
@@ -361,10 +361,20 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
return
}
go func() {
// Subscribe before reading the interval. The channel invalidates
// the interval, so any concurrent SetStaleAfter either happened
// before this interval read or will close thresholdCh below.
thresholdCh := svc.thresholdChan()
interval := svc.heartbeatInterval()
ticker := svc.clock.NewTicker(interval, "chatdebug", "heartbeat")
defer ticker.Stop()
thresholdCh := svc.thresholdChan()
resetTicker := func() {
if newInterval := svc.heartbeatInterval(); newInterval != interval {
interval = newInterval
ticker.Reset(interval, "chatdebug", "heartbeat")
}
}
for {
select {
case <-ctx.Done():
@@ -375,10 +385,7 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
// SetStaleAfter was called; re-read the interval
// and reset the ticker immediately.
thresholdCh = svc.thresholdChan()
if newInterval := svc.heartbeatInterval(); newInterval != interval {
interval = newInterval
ticker.Reset(interval, "chatdebug", "heartbeat")
}
resetTicker()
case <-ticker.C:
if err := svc.TouchStep(ctx, stepID, runID, chatID); err != nil {
svc.log.Debug(ctx, "heartbeat touch failed",
@@ -388,10 +395,7 @@ func launchHeartbeat(ctx context.Context, svc *Service, stepID, runID, chatID uu
}
// Also re-read interval on every tick as a
// secondary check.
if newInterval := svc.heartbeatInterval(); newInterval != interval {
interval = newInterval
ticker.Reset(interval, "chatdebug", "heartbeat")
}
resetTicker()
}
}
}()
@@ -1361,13 +1361,15 @@ func TestLaunchHeartbeat(t *testing.T) {
// threshold/2 interval.
newCall := tickerTrap.MustWait(ctx)
require.Equal(t, 30*time.Second, newCall.Duration)
// Reduce the threshold while NewTicker is trapped. This
// simulates SetStaleAfter racing with heartbeat startup before
// the goroutine can select on thresholdCh.
svc.SetStaleAfter(10 * time.Second)
newCall.MustRelease(ctx)
// Reducing the threshold must wake the heartbeat via the
// thresholdChan close and trigger a ticker reset to
// newThreshold/2 without advancing the mock clock.
svc.SetStaleAfter(10 * time.Second)
// The heartbeat must still reset to newThreshold/2 without
// advancing the mock clock.
resetCall := resetTrap.MustWait(ctx)
require.Equal(t, 5*time.Second, resetCall.Duration,
"ticker should reset to newThreshold/2 when SetStaleAfter"+