From b63eed94b6d30b3e73c75b6bd751b6b6fca202e8 Mon Sep 17 00:00:00 2001 From: Daniel Hiltgen Date: Fri, 31 Jul 2026 10:42:40 -0700 Subject: [PATCH] app/updater: drain background update-check goroutine before returning (#17446) DownloadNewRelease spawned a background checkForUpdate loop that read package-level knobs (UpdateCheckInterval et al.) and returned without waiting for it, so under -race the next test rewrote those globals while the orphaned goroutine was still reading them. waitDownloadIdle (from Cancel and WaitGroup-drain the loop before DownloadNewRelease returns, and have TestCancelOngoingDownload join its download goroutine so the drain is observable before the test exits. --- app/updater/updater.go | 12 +++++++++--- app/updater/updater_test.go | 6 ++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/updater/updater.go b/app/updater/updater.go index 06b3fd8db..e042807fe 100644 --- a/app/updater/updater.go +++ b/app/updater/updater.go @@ -153,10 +153,12 @@ func (u *Updater) DownloadNewRelease(ctx context.Context, updateResp UpdateRespo return err } - // In case of slow downloads, continue the update check in the background + // In case of slow downloads, continue the update check in the background. + // Drain the goroutine before returning: it reads package-level knobs + // (e.g. UpdateCheckInterval), which callers may mutate once we return. bgctx, bgcancel := context.WithCancel(downloadCtx) - defer bgcancel() - go func() { + var bgwg sync.WaitGroup + bgwg.Go(func() { for { select { case <-bgctx.Done(): @@ -165,6 +167,10 @@ func (u *Updater) DownloadNewRelease(ctx context.Context, updateResp UpdateRespo u.checkForUpdate(bgctx) } } + }) + defer func() { + bgcancel() + bgwg.Wait() }() resp, err := http.DefaultClient.Do(req) diff --git a/app/updater/updater_test.go b/app/updater/updater_test.go index fdca50375..7a27599cd 100644 --- a/app/updater/updater_test.go +++ b/app/updater/updater_test.go @@ -577,7 +577,9 @@ func TestCancelOngoingDownload(t *testing.T) { _, resp := updater.checkForUpdate(ctx) // Start download in goroutine + downloadDone := make(chan struct{}) go func() { + defer close(downloadDone) _ = updater.DownloadNewRelease(ctx, resp) }() @@ -598,6 +600,10 @@ func TestCancelOngoingDownload(t *testing.T) { case <-time.After(2 * time.Second): t.Fatal("download cancellation was not received by server") } + + // Wait for the download goroutine to unwind: it drags along a background + // update-check loop that reads package-level knobs the next test rewrites. + <-downloadDone } func TestTriggerImmediateCheck(t *testing.T) {