diff --git a/coderd/x/chatd/chatdebug/model.go b/coderd/x/chatd/chatdebug/model.go index 0ac7326080..e30a8a21e5 100644 --- a/coderd/x/chatd/chatdebug/model.go +++ b/coderd/x/chatd/chatdebug/model.go @@ -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() } } }() diff --git a/coderd/x/chatd/chatdebug/model_internal_test.go b/coderd/x/chatd/chatdebug/model_internal_test.go index a3386a7058..03bb51cab8 100644 --- a/coderd/x/chatd/chatdebug/model_internal_test.go +++ b/coderd/x/chatd/chatdebug/model_internal_test.go @@ -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"+