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>
This commit is contained in:
copilot-swe-agent[bot] 2026-04-01 00:50:15 +00:00 committed by yxxhero
parent 5ec606af3d
commit 3f6c982ae1
2 changed files with 15 additions and 24 deletions

View File

@ -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)

View File

@ -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)
}