Fix data-race during testing (#30999) (#31024)

Backport #30999 by wxiaoguang

Fix #30992

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/unit/unit.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync/atomic"
 
 	"code.gitea.io/gitea/models/perm"
 	"code.gitea.io/gitea/modules/container"
@@ -106,10 +107,23 @@ var (
 		TypeExternalTracker,
 	}
 
-	// DisabledRepoUnits contains the units that have been globally disabled
+	disabledRepoUnitsAtomic atomic.Pointer[[]Type] // the units that have been globally disabled
-	DisabledRepoUnits = []Type{}
 )
 
+// DisabledRepoUnitsGet returns the globally disabled units, it is a quick patch to fix data-race during testing.
+// Because the queue worker might read when a test is mocking the value. FIXME: refactor to a clear solution later.
+func DisabledRepoUnitsGet() []Type {
+	v := disabledRepoUnitsAtomic.Load()
+	if v == nil {
+		return nil
+	}
+	return *v
+}
+
+func DisabledRepoUnitsSet(v []Type) {
+	disabledRepoUnitsAtomic.Store(&v)
+}
+
 // Get valid set of default repository units from settings
 func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
 	units := defaultUnits
@@ -127,7 +141,7 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
 	}
 
 	// Remove disabled units
-	for _, disabledUnit := range DisabledRepoUnits {
+	for _, disabledUnit := range DisabledRepoUnitsGet() {
 		for i, unit := range units {
 			if unit == disabledUnit {
 				units = append(units[:i], units[i+1:]...)
@@ -140,11 +154,11 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
 
 // LoadUnitConfig load units from settings
 func LoadUnitConfig() error {
-	var invalidKeys []string
+	disabledRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DisabledRepoUnits...)
-	DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
 	if len(invalidKeys) > 0 {
 		log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
 	}
+	DisabledRepoUnitsSet(disabledRepoUnits)
 
 	setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
 	if len(invalidKeys) > 0 {
@@ -167,7 +181,7 @@ func LoadUnitConfig() error {
 
 // UnitGlobalDisabled checks if unit type is global disabled
 func (u Type) UnitGlobalDisabled() bool {
-	for _, ud := range DisabledRepoUnits {
+	for _, ud := range DisabledRepoUnitsGet() {
 		if u == ud {
 			return true
 		}

models/unit/unit_test.go

@@ -14,10 +14,10 @@ import (
 func TestLoadUnitConfig(t *testing.T) {
 	t.Run("regular", func(t *testing.T) {
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
-			DisabledRepoUnits = disabledRepoUnits
+			DisabledRepoUnitsSet(disabledRepoUnits)
 			DefaultRepoUnits = defaultRepoUnits
 			DefaultForkRepoUnits = defaultForkRepoUnits
-		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
 			setting.Repository.DisabledRepoUnits = disabledRepoUnits
 			setting.Repository.DefaultRepoUnits = defaultRepoUnits
@@ -28,16 +28,16 @@ func TestLoadUnitConfig(t *testing.T) {
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
 		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
 		assert.NoError(t, LoadUnitConfig())
-		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
 		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 	t.Run("invalid", func(t *testing.T) {
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
-			DisabledRepoUnits = disabledRepoUnits
+			DisabledRepoUnitsSet(disabledRepoUnits)
 			DefaultRepoUnits = defaultRepoUnits
 			DefaultForkRepoUnits = defaultForkRepoUnits
-		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
 			setting.Repository.DisabledRepoUnits = disabledRepoUnits
 			setting.Repository.DefaultRepoUnits = defaultRepoUnits
@@ -48,16 +48,16 @@ func TestLoadUnitConfig(t *testing.T) {
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"}
 		setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
 		assert.NoError(t, LoadUnitConfig())
-		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
 		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 	t.Run("duplicate", func(t *testing.T) {
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
-			DisabledRepoUnits = disabledRepoUnits
+			DisabledRepoUnitsSet(disabledRepoUnits)
 			DefaultRepoUnits = defaultRepoUnits
 			DefaultForkRepoUnits = defaultForkRepoUnits
-		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
 			setting.Repository.DisabledRepoUnits = disabledRepoUnits
 			setting.Repository.DefaultRepoUnits = defaultRepoUnits
@@ -68,16 +68,16 @@ func TestLoadUnitConfig(t *testing.T) {
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"}
 		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
 		assert.NoError(t, LoadUnitConfig())
-		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
 		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 	t.Run("empty_default", func(t *testing.T) {
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
-			DisabledRepoUnits = disabledRepoUnits
+			DisabledRepoUnitsSet(disabledRepoUnits)
 			DefaultRepoUnits = defaultRepoUnits
 			DefaultForkRepoUnits = defaultForkRepoUnits
-		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
 		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
 			setting.Repository.DisabledRepoUnits = disabledRepoUnits
 			setting.Repository.DefaultRepoUnits = defaultRepoUnits
@@ -88,7 +88,7 @@ func TestLoadUnitConfig(t *testing.T) {
 		setting.Repository.DefaultRepoUnits = []string{}
 		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
 		assert.NoError(t, LoadUnitConfig())
-		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
 		assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects, TypeActions}, DefaultRepoUnits)
 		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})

tests/integration/org_project_test.go

@@ -9,13 +9,15 @@ import (
 	"testing"
 
 	unit_model "code.gitea.io/gitea/models/unit"
-	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/tests"
 )
 
 func TestOrgProjectAccess(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
-	defer test.MockVariableValue(&unit_model.DisabledRepoUnits, append(slices.Clone(unit_model.DisabledRepoUnits), unit_model.TypeProjects))()
+
+	disabledRepoUnits := unit_model.DisabledRepoUnitsGet()
+	unit_model.DisabledRepoUnitsSet(append(slices.Clone(disabledRepoUnits), unit_model.TypeProjects))
+	defer unit_model.DisabledRepoUnitsSet(disabledRepoUnits)
 
 	// repo project, 404
 	req := NewRequest(t, "GET", "/user2/repo1/projects")