From e89bb56b62888c3f209673c69d391be8adc14a60 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Thu, 27 Nov 2025 01:32:53 +0100 Subject: [PATCH] fix: add double-check locking for in-memory chart cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple workers concurrently process releases using the same chart, they all check the in-memory cache before acquiring locks. If none have populated the cache yet, all workers miss and try to download. Previously, even after acquiring the exclusive lock, the code would re-download the chart when needsRefresh=true (the default). This caused multiple "Pulling" messages in tests like oci_chart_pull_once. The fix adds a second in-memory cache check AFTER acquiring the lock. This implements proper double-check locking: 1. Check cache (outside lock) → miss 2. Acquire lock 3. Check cache again (inside lock) → hit if another worker populated it 4. If still miss, download and add to cache This ensures only one worker downloads the chart, while others use the cached version populated by the first worker. Changes: - Add in-memory cache double-check in getOCIChart() after acquiring lock - Add in-memory cache double-check in forcedDownloadChart() after acquiring lock This fixes the oci_chart_pull_once and oci_chart_pull_direct test failures where charts were being pulled multiple times instead of once. Signed-off-by: Aditya Menon --- pkg/state/state.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index d50816d2..00e15c2b 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1484,7 +1484,15 @@ func (st *HelmState) forcedDownloadChart(chartName, dir string, release *Release return "", err } - // If cached, release lock and return + // Double-check in-memory cache after acquiring lock + // Another worker in this process might have populated the cache while we waited + if cachedPath, exists := st.checkChartCache(cacheKey); exists && st.fs.DirectoryExistsAt(cachedPath) { + st.logger.Debugf("Chart %s:%s found in cache after acquiring lock, using cached version at %s", chartName, release.Version, cachedPath) + lockResult.Release(st.logger) + return cachedPath, nil + } + + // If cached on filesystem, release lock and return if lockResult.action == chartActionUseCached { st.addToChartCache(cacheKey, lockResult.cachedPath) lockResult.Release(st.logger) @@ -4611,7 +4619,15 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm return nil, err } - // If cached, release lock and return + // Double-check in-memory cache after acquiring lock + // Another worker in this process might have populated the cache while we waited + if cachedPath, exists := st.checkChartCache(cacheKey); exists && st.fs.DirectoryExistsAt(cachedPath) { + st.logger.Debugf("OCI chart %s:%s found in cache after acquiring lock, using cached version at %s", chartName, chartVersion, cachedPath) + lockResult.Release(st.logger) + return &cachedPath, nil + } + + // If cached on filesystem, release lock and return if lockResult.action == chartActionUseCached { st.addToChartCache(cacheKey, lockResult.cachedPath) lockResult.Release(st.logger)