Browse Source

Fix status table race condition (#1835)

Ethan Koenig 3 years ago
parent
commit
bfb44f8854
5 changed files with 26 additions and 10 deletions
  1. 3 6
      models/repo.go
  2. 1 2
      models/repo_mirror.go
  3. 1 2
      models/user.go
  4. 12 0
      modules/sync/status_pool.go
  5. 9 0
      modules/sync/status_pool_test.go

+ 3 - 6
models/repo.go

@@ -1881,10 +1881,9 @@ func DeleteRepositoryArchives() error {
1881 1881
 
1882 1882
 // DeleteOldRepositoryArchives deletes old repository archives.
1883 1883
 func DeleteOldRepositoryArchives() {
1884
-	if taskStatusTable.IsRunning(archiveCleanup) {
1884
+	if !taskStatusTable.StartIfNotRunning(archiveCleanup) {
1885 1885
 		return
1886 1886
 	}
1887
-	taskStatusTable.Start(archiveCleanup)
1888 1887
 	defer taskStatusTable.Stop(archiveCleanup)
1889 1888
 
1890 1889
 	log.Trace("Doing: ArchiveCleanup")
@@ -2025,10 +2024,9 @@ const (
2025 2024
 
2026 2025
 // GitFsck calls 'git fsck' to check repository health.
2027 2026
 func GitFsck() {
2028
-	if taskStatusTable.IsRunning(gitFsck) {
2027
+	if !taskStatusTable.StartIfNotRunning(gitFsck) {
2029 2028
 		return
2030 2029
 	}
2031
-	taskStatusTable.Start(gitFsck)
2032 2030
 	defer taskStatusTable.Stop(gitFsck)
2033 2031
 
2034 2032
 	log.Trace("Doing: GitFsck")
@@ -2097,10 +2095,9 @@ func repoStatsCheck(checker *repoChecker) {
2097 2095
 
2098 2096
 // CheckRepoStats checks the repository stats
2099 2097
 func CheckRepoStats() {
2100
-	if taskStatusTable.IsRunning(checkRepos) {
2098
+	if !taskStatusTable.StartIfNotRunning(checkRepos) {
2101 2099
 		return
2102 2100
 	}
2103
-	taskStatusTable.Start(checkRepos)
2104 2101
 	defer taskStatusTable.Stop(checkRepos)
2105 2102
 
2106 2103
 	log.Trace("Doing: CheckRepoStats")

+ 1 - 2
models/repo_mirror.go

@@ -202,10 +202,9 @@ func DeleteMirrorByRepoID(repoID int64) error {
202 202
 
203 203
 // MirrorUpdate checks and updates mirror repositories.
204 204
 func MirrorUpdate() {
205
-	if taskStatusTable.IsRunning(mirrorUpdate) {
205
+	if !taskStatusTable.StartIfNotRunning(mirrorUpdate) {
206 206
 		return
207 207
 	}
208
-	taskStatusTable.Start(mirrorUpdate)
209 208
 	defer taskStatusTable.Stop(mirrorUpdate)
210 209
 
211 210
 	log.Trace("Doing: MirrorUpdate")

+ 1 - 2
models/user.go

@@ -1334,10 +1334,9 @@ func GetWatchedRepos(userID int64, private bool) ([]*Repository, error) {
1334 1334
 
1335 1335
 // SyncExternalUsers is used to synchronize users with external authorization source
1336 1336
 func SyncExternalUsers() {
1337
-	if taskStatusTable.IsRunning(syncExternalUsers) {
1337
+	if !taskStatusTable.StartIfNotRunning(syncExternalUsers) {
1338 1338
 		return
1339 1339
 	}
1340
-	taskStatusTable.Start(syncExternalUsers)
1341 1340
 	defer taskStatusTable.Stop(syncExternalUsers)
1342 1341
 
1343 1342
 	log.Trace("Doing: SyncExternalUsers")

+ 12 - 0
modules/sync/status_pool.go

@@ -24,6 +24,18 @@ func NewStatusTable() *StatusTable {
24 24
 	}
25 25
 }
26 26
 
27
+// StartIfNotRunning sets value of given name to true if not already in pool.
28
+// Returns whether set value was set to true
29
+func (p *StatusTable) StartIfNotRunning(name string) bool {
30
+	p.lock.Lock()
31
+	_, ok := p.pool[name]
32
+	if !ok {
33
+		p.pool[name] = struct{}{}
34
+	}
35
+	p.lock.Unlock()
36
+	return !ok
37
+}
38
+
27 39
 // Start sets value of given name to true in the pool.
28 40
 func (p *StatusTable) Start(name string) {
29 41
 	p.lock.Lock()

+ 9 - 0
modules/sync/status_pool_test.go

@@ -18,6 +18,15 @@ func Test_StatusTable(t *testing.T) {
18 18
 	table.Start("xyz")
19 19
 	assert.True(t, table.IsRunning("xyz"))
20 20
 
21
+	assert.False(t, table.StartIfNotRunning("xyz"))
22
+	assert.True(t, table.IsRunning("xyz"))
23
+
24
+	table.Stop("xyz")
25
+	assert.False(t, table.IsRunning("xyz"))
26
+
27
+	assert.True(t, table.StartIfNotRunning("xyz"))
28
+	assert.True(t, table.IsRunning("xyz"))
29
+
21 30
 	table.Stop("xyz")
22 31
 	assert.False(t, table.IsRunning("xyz"))
23 32
 }