From 3f6c982ae1258c882c470650cc3d5a41cedbf098 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 00:50:15 +0000 Subject: [PATCH] fix: remove duplicate mutex, reuse getNamedRWMutex, restore on write failure, add test barrier Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f93746bf-82fa-46b1-b8f0-b34bc9aa749c Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/chart_dependencies_rewrite_test.go | 10 ++++++- pkg/state/state.go | 29 ++++---------------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/pkg/state/chart_dependencies_rewrite_test.go b/pkg/state/chart_dependencies_rewrite_test.go index e917c76f..1792ff81 100644 --- a/pkg/state/chart_dependencies_rewrite_test.go +++ b/pkg/state/chart_dependencies_rewrite_test.go @@ -527,16 +527,21 @@ dependencies: t.Fatalf("failed to write Chart.yaml: %v", err) } - // Run multiple goroutines concurrently on the same chart path + // Run multiple goroutines concurrently on the same chart path. + // A start barrier ensures all goroutines attempt rewriteChartDependencies simultaneously. numGoroutines := 10 var wg sync.WaitGroup errCh := make(chan error, numGoroutines) + ready := make(chan struct{}) for i := 0; i < numGoroutines; i++ { wg.Add(1) go func() { defer wg.Done() + // Wait until all goroutines are ready so they start simultaneously. + <-ready + logger := zap.NewNop().Sugar() st := &HelmState{ logger: logger, @@ -552,6 +557,9 @@ dependencies: }() } + // Release all goroutines at once to maximize contention. + close(ready) + wg.Wait() close(errCh) diff --git a/pkg/state/state.go b/pkg/state/state.go index 19155b65..d48b664b 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1407,28 +1407,6 @@ type PrepareChartKey struct { Namespace, Name, KubeContext string } -// PrepareCharts creates temporary directories of charts. -// -// Each resulting "chart" can be one of the followings: -// -// (1) local chart -// (2) temporary local chart generated from kustomization or manifests -// (3) remote chart -// -// When running `helmfile template` on helm v2, or `helmfile lint` on both helm v2 and v3, -// PrepareCharts will download and untar charts for linting and templating. -var chartDepsMutexMap sync.Map - -func (st *HelmState) getChartDepsMutex(chartPath string) *sync.Mutex { - mu, ok := chartDepsMutexMap.Load(chartPath) - if ok { - return mu.(*sync.Mutex) - } - newMu := &sync.Mutex{} - actualMu, _ := chartDepsMutexMap.LoadOrStore(chartPath, newMu) - return actualMu.(*sync.Mutex) -} - // rewriteChartDependencies rewrites relative file:// dependencies in Chart.yaml to absolute paths // to ensure they can be resolved from chartify's temporary directory func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) { @@ -1439,7 +1417,8 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) return func() {}, nil } - mu := st.getChartDepsMutex(chartPath) + // Reuse the existing per-path RWMutex for exclusive access to the chart directory. + mu := st.getNamedRWMutex(chartPath) mu.Lock() // Read Chart.yaml @@ -1508,6 +1487,10 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) } if err := os.WriteFile(chartYamlPath, updatedData, 0644); err != nil { + // File may be truncated/partially written; restore original content before releasing the lock. + if restoreErr := os.WriteFile(chartYamlPath, originalContent, 0644); restoreErr != nil { + st.logger.Warnf("Failed to restore Chart.yaml at %s after write error (%v): %v", chartYamlPath, err, restoreErr) + } mu.Unlock() return func() {}, fmt.Errorf("failed to write Chart.yaml: %w", err) }