From 4f5a3e13e5f3aca85d830e3a9024c893b0bbf1e1 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sun, 1 Nov 2026 01:20:24 -0700 Subject: [PATCH] fix(repo): don't get stuck generating contributor stats (#40125) (#40156) Backport #40125 by @silverwind A failed contributor stats generation never released its lock, so after the cached error expired, the contributors and code frequency pages kept saying stats are being generated until a restart. A generation outlasting the 5 second wait also leaked its goroutine on an unread channel. The lock is now claimed atomically and released together with the completion signal when generation ends, and a cache miss after the wait counts as still generating. Co-authored-by: silverwind --- services/repository/contributors_graph.go | 18 +++++++----------- services/repository/contributors_graph_test.go | 15 +++++++-------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/services/repository/contributors_graph.go b/services/repository/contributors_graph.go index 6211d53feef..1b9e9c4d9a8 100644 --- a/services/repository/contributors_graph.go +++ b/services/repository/contributors_graph.go @@ -81,16 +81,12 @@ func GetContributorStats(ctx context.Context, cache cache.StringCache, repo *rep // as GetContributorStats is resource intensive we cache the result cacheKey := fmt.Sprintf(contributorStatsCacheKey, repo.FullName(), revision) if !cache.IsExist(cacheKey) { - genReady := make(chan struct{}) - // don't start multiple async generations - _, run := generateLock.Load(cacheKey) - if run { + if _, running := generateLock.LoadOrStore(cacheKey, struct{}{}); running { return nil, ErrAwaitGeneration } - generateLock.Store(cacheKey, struct{}{}) - // run generation async + genReady := make(chan struct{}) go generateContributorStats(genReady, cache, cacheKey, repo, revision) select { @@ -103,8 +99,10 @@ func GetContributorStats(ctx context.Context, cache cache.StringCache, repo *rep } // TODO: renew timeout of cache cache.UpdateTimeout(cacheKey, contributorStatsCacheTimeout) var res map[string]*ContributorData - if _, cacheErr := cache.GetJSON(cacheKey, &res); cacheErr != nil { + if exist, cacheErr := cache.GetJSON(cacheKey, &res); cacheErr != nil { return nil, fmt.Errorf("cached error: %w", cacheErr.ToError()) + } else if !exist { + return nil, ErrAwaitGeneration } return res, nil } @@ -188,6 +186,8 @@ func getExtendedCommitStats(ctx context.Context, repo *git.Repository, revision } func generateContributorStats(genDone chan struct{}, cache cache.StringCache, cacheKey string, repo *repo_model.Repository, revision string) { + defer close(genDone) + defer generateLock.Delete(cacheKey) ctx := graceful.GetManager().HammerContext() gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, repo) @@ -289,8 +289,4 @@ func generateContributorStats(genDone chan struct{}, cache cache.StringCache, ca } _ = cache.PutJSON(cacheKey, contributorsCommitStats, contributorStatsCacheTimeout) - generateLock.Delete(cacheKey) - if genDone != nil { - genDone <- struct{}{} - } } diff --git a/services/repository/contributors_graph_test.go b/services/repository/contributors_graph_test.go index fb9695c1d96..7fb5895fefa 100644 --- a/services/repository/contributors_graph_test.go +++ b/services/repository/contributors_graph_test.go @@ -4,6 +4,7 @@ package repository import ( + "fmt" "slices" "testing" @@ -23,15 +24,13 @@ func TestRepository_ContributorsGraph(t *testing.T) { mockCache, err := cache.NewStringCache(setting.Cache{}) assert.NoError(t, err) - generateContributorStats(nil, mockCache, "key", repo, "404ref") - var data map[string]*ContributorData - _, getErr := mockCache.GetJSON("key", &data) - assert.NotNil(t, getErr) - assert.ErrorContains(t, getErr.ToError(), "object does not exist") + _, err = GetContributorStats(t.Context(), mockCache, repo, "404ref") + assert.ErrorContains(t, err, "object does not exist") + _, locked := generateLock.Load(fmt.Sprintf(contributorStatsCacheKey, repo.FullName(), "404ref")) + assert.False(t, locked) - generateContributorStats(nil, mockCache, "key2", repo, "master") - exist, _ := mockCache.GetJSON("key2", &data) - assert.True(t, exist) + data, err := GetContributorStats(t.Context(), mockCache, repo, "master") + assert.NoError(t, err) var keys []string for k := range data { keys = append(keys, k)