chore: Update cfg locking for units (#6401)

* chore: Updating locking logic for units to mitigate a race

* chore: Adding `GuardConfigParse`

* chore: Cleaning up some commentary

Author: yhakbar

internal/component/component.go

@@ -43,8 +43,6 @@ type Component interface {
 
 	lock()
 	unlock()
-	rLock()
-	rUnlock()
 
 	ensureDependency(Component)
 	ensureDependent(Component)

internal/component/component_test.go

@@ -2,9 +2,11 @@ package component_test
 
 import (
 	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/gruntwork-io/terragrunt/internal/component"
+	"github.com/gruntwork-io/terragrunt/pkg/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -235,3 +237,34 @@ func TestThreadSafeComponentsConcurrentAccess(t *testing.T) {
 	// Should have exactly one component despite concurrent adds
 	assert.Equal(t, 1, tsc.Len(), "should have exactly one component after concurrent adds")
 }
+
+// TestUnitGuardConfigParseWithRacing verifies the parse runs exactly once when
+// many goroutines race to parse the same unit.
+func TestUnitGuardConfigParseWithRacing(t *testing.T) {
+	t.Parallel()
+
+	unit := component.NewUnit("/test/unit")
+
+	var parses atomic.Int32
+
+	var wg sync.WaitGroup
+
+	const goroutines = 32
+
+	for range goroutines {
+		wg.Go(func() {
+			err := unit.GuardConfigParse(func() error {
+				parses.Add(1)
+				unit.StoreConfig(&config.TerragruntConfig{})
+
+				return nil
+			})
+			assert.NoError(t, err)
+		})
+	}
+
+	wg.Wait()
+
+	assert.Equal(t, int32(1), parses.Load(), "config should be parsed exactly once")
+	assert.NotNil(t, unit.Config(), "config should be populated after parsing")
+}

internal/component/stack.go

@@ -24,7 +24,7 @@ type Stack struct {
 	dependencies     Components
 	dependents       Components
 	Units            []*Unit
-	mu               sync.RWMutex
+	mu               sync.Mutex
 	external         bool
 }
 
@@ -47,11 +47,17 @@ func (s *Stack) WithDiscoveryContext(ctx *DiscoveryContext) *Stack {
 
 // Config returns the parsed Stack configuration for this stack.
 func (s *Stack) Config() *config.StackConfig {
+	s.lock()
+	defer s.unlock()
+
 	return s.cfg
 }
 
 // StoreConfig stores the parsed Stack configuration for this stack.
 func (s *Stack) StoreConfig(cfg *config.StackConfig) {
+	s.lock()
+	defer s.unlock()
+
 	s.cfg = cfg
 }
 
@@ -96,11 +102,17 @@ func (s *Stack) SetExternal() {
 
 // Reading returns the list of files being read by this component.
 func (s *Stack) Reading() []string {
+	s.lock()
+	defer s.unlock()
+
 	return s.reading
 }
 
 // SetReading sets the list of files being read by this component.
 func (s *Stack) SetReading(files ...string) {
+	s.lock()
+	defer s.unlock()
+
 	s.reading = files
 }
 
@@ -145,16 +157,6 @@ func (s *Stack) unlock() {
 	s.mu.Unlock()
 }
 
-// rLock locks the Stack for reading.
-func (s *Stack) rLock() {
-	s.mu.RLock()
-}
-
-// rUnlock unlocks the Stack for reading.
-func (s *Stack) rUnlock() {
-	s.mu.RUnlock()
-}
-
 // AddDependency adds a dependency to the Stack and vice versa.
 //
 // Using this method ensure that the dependency graph is properly maintained,
@@ -199,16 +201,16 @@ func (s *Stack) AddDependent(dependent Component) {
 
 // Dependencies returns the dependencies of the Stack.
 func (s *Stack) Dependencies() Components {
-	s.rLock()
-	defer s.rUnlock()
+	s.lock()
+	defer s.unlock()
 
 	return s.dependencies
 }
 
 // Dependents returns the dependents of the Stack.
 func (s *Stack) Dependents() Components {
-	s.rLock()
-	defer s.rUnlock()
+	s.lock()
+	defer s.unlock()
 
 	return s.dependents
 }

internal/component/unit.go

@@ -25,7 +25,8 @@ type Unit struct {
 	reading          []string
 	dependencies     Components
 	dependents       Components
-	mu               sync.RWMutex
+	mu               sync.Mutex
+	parseMu          sync.Mutex
 	external         bool
 	excluded         bool
 }
@@ -51,7 +52,7 @@ func (u *Unit) WithReading(files ...string) *Unit {
 
 // WithConfig adds configuration to a Unit component.
 func (u *Unit) WithConfig(cfg *config.TerragruntConfig) *Unit {
-	u.cfg = cfg
+	u.StoreConfig(cfg)
 
 	return u
 }
@@ -65,14 +66,37 @@ func (u *Unit) WithDiscoveryContext(ctx *DiscoveryContext) *Unit {
 
 // Config returns the parsed Terragrunt configuration for this unit.
 func (u *Unit) Config() *config.TerragruntConfig {
+	u.lock()
+	defer u.unlock()
+
 	return u.cfg
 }
 
 // StoreConfig stores the parsed Terragrunt configuration for this unit.
 func (u *Unit) StoreConfig(cfg *config.TerragruntConfig) {
+	u.lock()
+	defer u.unlock()
+
 	u.cfg = cfg
 }
 
+// GuardConfigParse runs parse to populate this unit's config at most once, even
+// when called concurrently. Callers that lose the race skip parse and observe
+// the config stored by the winner.
+//
+// The guard is separate from the config mutex so a slow parse doesn't block
+// reads of an already-parsed unit.
+func (u *Unit) GuardConfigParse(parse func() error) error {
+	u.parseMu.Lock()
+	defer u.parseMu.Unlock()
+
+	if u.Config() != nil {
+		return nil
+	}
+
+	return parse()
+}
+
 // ConfigFile returns the discovered config filename for this unit.
 func (u *Unit) ConfigFile() string {
 	return u.configFile
@@ -120,16 +144,25 @@ func (u *Unit) SetExcluded(excluded bool) {
 
 // Reading returns the list of files being read by this component.
 func (u *Unit) Reading() []string {
+	u.lock()
+	defer u.unlock()
+
 	return u.reading
 }
 
 // SetReading sets the list of files being read by this component.
 func (u *Unit) SetReading(files ...string) {
+	u.lock()
+	defer u.unlock()
+
 	u.reading = files
 }
 
 // Sources returns the list of sources for this component.
 func (u *Unit) Sources() []string {
+	u.lock()
+	defer u.unlock()
+
 	if u.cfg == nil || u.cfg.Terraform == nil || u.cfg.Terraform.Source == nil {
 		return []string{}
 	}
@@ -166,16 +199,6 @@ func (u *Unit) unlock() {
 	u.mu.Unlock()
 }
 
-// rLock locks the Unit for reading.
-func (u *Unit) rLock() {
-	u.mu.RLock()
-}
-
-// rUnlock unlocks the Unit for reading.
-func (u *Unit) rUnlock() {
-	u.mu.RUnlock()
-}
-
 // AddDependency adds a dependency to the Unit and vice versa.
 //
 // Using this method ensure that the dependency graph is properly maintained,
@@ -220,16 +243,16 @@ func (u *Unit) AddDependent(dependent Component) {
 
 // Dependencies returns the dependencies of the Unit.
 func (u *Unit) Dependencies() Components {
-	u.rLock()
-	defer u.rUnlock()
+	u.lock()
+	defer u.unlock()
 
 	return u.dependencies
 }
 
 // Dependents returns the dependents of the Unit.
 func (u *Unit) Dependents() Components {
-	u.rLock()
-	defer u.rUnlock()
+	u.lock()
+	defer u.unlock()
 
 	return u.dependents
 }
@@ -240,9 +263,9 @@ func (u *Unit) Dependents() Components {
 //
 //	Unit /path/to/unit (excluded: false, assume applied: false, dependencies: [/dep1, /dep2])
 func (u *Unit) String() string {
-	// Snapshot values under read lock to avoid data races
-	u.rLock()
-	defer u.rUnlock()
+	// Snapshot values under lock to avoid data races
+	u.lock()
+	defer u.unlock()
 
 	path := u.DisplayPath()
 	deps := make([]string, 0, len(u.dependencies))

internal/discovery/concurrent_parse_race_test.go

@@ -0,0 +1,81 @@
+package discovery_test
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/gruntwork-io/terragrunt/internal/component"
+	"github.com/gruntwork-io/terragrunt/internal/discovery"
+	"github.com/gruntwork-io/terragrunt/internal/filter"
+	"github.com/gruntwork-io/terragrunt/pkg/options"
+	"github.com/gruntwork-io/terragrunt/test/helpers"
+	"github.com/gruntwork-io/terragrunt/test/helpers/logger"
+	"github.com/stretchr/testify/require"
+)
+
+// TestDiscovery_GraphConcurrentConfigAccessWithRacing reproduces, through the
+// public Discover entry point, the data race the per-Unit cfg/reading locks
+// guard against: the graph phase reaches a shared unit from several goroutines
+// at once, so one goroutine's parse stores the config while others read it.
+//
+// The shared unit gets a large config and many dependents, repeated across
+// several Discover calls, so the read/write overlap is reliably observable; a
+// smaller config or fewer iterations make the race intermittent.
+//
+// To confirm the locks are load-bearing, drop the lock/unlock calls from Unit's
+// Config, StoreConfig, Reading, and SetReading and run with -race.
+func TestDiscovery_GraphConcurrentConfigAccessWithRacing(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := helpers.TmpDirWOSymlinks(t)
+
+	// remote_state is partially decoded during discovery, so a large block is
+	// walked during parse rather than skipped, which lengthens the parse.
+	var sharedConfig strings.Builder
+
+	sharedConfig.WriteString("remote_state {\n  backend = \"local\"\n")
+	sharedConfig.WriteString("  generate = { path = \"backend.tf\", if_exists = \"overwrite\" }\n  config = {\n")
+
+	for i := range 8000 {
+		fmt.Fprintf(&sharedConfig, "    k%d = \"v%d\"\n", i, i)
+	}
+
+	sharedConfig.WriteString("  }\n}\n")
+
+	vpcDir := filepath.Join(tmpDir, "vpc")
+	require.NoError(t, os.MkdirAll(vpcDir, 0755))
+	require.NoError(t, os.WriteFile(filepath.Join(vpcDir, "terragrunt.hcl"), []byte(sharedConfig.String()), 0644))
+
+	const leaves = 8
+
+	for i := range leaves {
+		leafDir := filepath.Join(tmpDir, fmt.Sprintf("app%d", i))
+		require.NoError(t, os.MkdirAll(leafDir, 0755))
+		require.NoError(t, os.WriteFile(filepath.Join(leafDir, "terragrunt.hcl"), []byte(`
+		dependency "vpc" {
+			config_path = "../vpc"
+		}
+		`), 0644))
+	}
+
+	l := logger.CreateLogger()
+	opts := &options.TerragruntOptions{
+		WorkingDir:     tmpDir,
+		RootWorkingDir: tmpDir,
+	}
+
+	filters, err := filter.ParseFilterQueries(l, []string{"{./**}..."})
+	require.NoError(t, err)
+
+	for range 8 {
+		d := discovery.NewDiscovery(tmpDir).
+			WithDiscoveryContext(&component.DiscoveryContext{WorkingDir: tmpDir}).
+			WithFilters(filters)
+
+		_, err := d.Discover(t.Context(), l, opts)
+		require.NoError(t, err)
+	}
+}

internal/discovery/phase_parse.go

@@ -124,7 +124,9 @@ func ensureParsed(
 		return nil
 	}
 
-	return parseComponent(ctx, l, c, opts, discovery)
+	return unit.GuardConfigParse(func() error {
+		return parseComponent(ctx, l, c, opts, discovery)
+	})
 }
 
 // ParsePhase parses HCL configurations for filter evaluation.