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 <me@silverwind.io>

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{}{}
-	}
 }

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)