From 5e5b4d7054ca913499b82e7dc5742d6c66970655 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Thu, 3 Sep 2026 08:18:46 +0000 Subject: [PATCH] make cycleDetector.stopped atomic Fixes the flaky build-darwin job and adds a regression test for the race with `go state.checkForCycles()` in `src/core/state.go`. The main goroutine can call Stop while the async check is in flight. --- src/core/cycle_detector.go | 11 ++++++----- src/core/cycle_detector_test.go | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/core/cycle_detector.go b/src/core/cycle_detector.go index 39dc99549e..af808cb2bb 100644 --- a/src/core/cycle_detector.go +++ b/src/core/cycle_detector.go @@ -3,17 +3,18 @@ package core import ( "fmt" "strings" + "sync/atomic" ) type cycleDetector struct { graph *BuildGraph - stopped bool + stopped atomic.Bool } // Check runs a single check of the build graph to see if any cycles can be detected. // If it finds one an errCycle is returned. func (c *cycleDetector) Check() *errCycle { - if c.stopped { + if c.stopped.Load() { return nil } log.Debug("Running cycle detection...") @@ -27,7 +28,7 @@ func (c *cycleDetector) Check() *errCycle { // cycle is complete or not (if not the caller will need to add its node to it as well). var visit func(target *BuildTarget) ([]*BuildTarget, bool) visit = func(target *BuildTarget) ([]*BuildTarget, bool) { - if c.stopped { + if c.stopped.Load() { return nil, false } else if _, present := complete[target]; present { return nil, false @@ -49,7 +50,7 @@ func (c *cycleDetector) Check() *errCycle { } for _, target := range c.graph.AllTargets() { - if c.stopped { + if c.stopped.Load() { log.Debug("Cycle detection terminated") return nil } @@ -66,7 +67,7 @@ func (c *cycleDetector) Check() *errCycle { // Stop stops any existing run of the cycle detector. func (c *cycleDetector) Stop() { - c.stopped = true + c.stopped.Store(true) } // An errCycle is emitted when a graph cycle is detected. diff --git a/src/core/cycle_detector_test.go b/src/core/cycle_detector_test.go index 70506f59b7..241c373f2c 100644 --- a/src/core/cycle_detector_test.go +++ b/src/core/cycle_detector_test.go @@ -1,6 +1,7 @@ package core import ( + "fmt" "testing" "time" @@ -71,4 +72,22 @@ func TestCycleDetector(t *testing.T) { log.Warning("%s", err) assert.Equal(t, []*BuildTarget{g, e, f}, err.Cycle) }) + + // This is a regression test for a race with `go state.checkForCycles()` + // in `src/core/state.go`. The main goroutine can call Stop while the + // async check is in flight. + t.Run("StopDuringCheck", func(t *testing.T) { + state := NewDefaultBuildState() + for i := 0; i < 100; i++ { + state.Graph.AddTarget(NewBuildTarget(ParseBuildLabel(fmt.Sprintf("//src:t%d", i), ""))) + } + detector := cycleDetector{graph: state.Graph} + done := make(chan struct{}) + go func() { + defer close(done) + assert.Nil(t, detector.Check()) + }() + detector.Stop() + <-done + }) }