From e83abfc2898e499c64276fa1bf03e6ccf5aa0390 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Sat, 17 Jul 2021 18:09:56 +0100
Subject: [PATCH] Prevent race in TestPersistableChannelQueue (#16468)

* Prevent race in TestPersistableChannelQueue

A slight race has become apparent in the TestPersistableChannelQueue.

This PR simply adds locking to prevent the race.

* make print value of "$(GOTESTFLAGS)" on test-backend and unit-test-coverage


Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
---
 Makefile                                 |  4 +--
 modules/queue/queue_disk_channel_test.go | 35 +++++++++++++++++++++---
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index c07dfbf475..a0f5fdab30 100644
--- a/Makefile
+++ b/Makefile
@@ -359,7 +359,7 @@ test: test-frontend test-backend
 
 .PHONY: test-backend
 test-backend:
-	@echo "Running go test with -tags '$(TEST_TAGS)'..."
+	@echo "Running go test with $(GOTESTFLAGS) -tags '$(TEST_TAGS)'..."
 	@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='$(TEST_TAGS)' $(GO_PACKAGES)
 
 .PHONY: test-frontend
@@ -389,7 +389,7 @@ coverage:
 
 .PHONY: unit-test-coverage
 unit-test-coverage:
-	@echo "Running unit-test-coverage -tags '$(TEST_TAGS)'..."
+	@echo "Running unit-test-coverage $(GOTESTFLAGS) -tags '$(TEST_TAGS)'..."
 	@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='$(TEST_TAGS)' -cover -coverprofile coverage.out $(GO_PACKAGES) && echo "\n==>\033[32m Ok\033[m\n" || exit 1
 
 .PHONY: vendor
diff --git a/modules/queue/queue_disk_channel_test.go b/modules/queue/queue_disk_channel_test.go
index 561f98ca90..bbe8a5ecbd 100644
--- a/modules/queue/queue_disk_channel_test.go
+++ b/modules/queue/queue_disk_channel_test.go
@@ -6,6 +6,7 @@ package queue
 
 import (
 	"io/ioutil"
+	"sync"
 	"testing"
 
 	"code.gitea.io/gitea/modules/util"
@@ -22,6 +23,7 @@ func TestPersistableChannelQueue(t *testing.T) {
 		}
 	}
 
+	lock := sync.Mutex{}
 	queueShutdown := []func(){}
 	queueTerminate := []func(){}
 
@@ -41,8 +43,12 @@ func TestPersistableChannelQueue(t *testing.T) {
 	assert.NoError(t, err)
 
 	go queue.Run(func(shutdown func()) {
+		lock.Lock()
+		defer lock.Unlock()
 		queueShutdown = append(queueShutdown, shutdown)
 	}, func(terminate func()) {
+		lock.Lock()
+		defer lock.Unlock()
 		queueTerminate = append(queueTerminate, terminate)
 	})
 
@@ -69,7 +75,11 @@ func TestPersistableChannelQueue(t *testing.T) {
 	assert.Error(t, err)
 
 	// Now shutdown the queue
-	for _, callback := range queueShutdown {
+	lock.Lock()
+	callbacks := make([]func(), len(queueShutdown))
+	copy(callbacks, queueShutdown)
+	lock.Unlock()
+	for _, callback := range callbacks {
 		callback()
 	}
 
@@ -87,7 +97,11 @@ func TestPersistableChannelQueue(t *testing.T) {
 	}
 
 	// terminate the queue
-	for _, callback := range queueTerminate {
+	lock.Lock()
+	callbacks = make([]func(), len(queueTerminate))
+	copy(callbacks, queueTerminate)
+	lock.Unlock()
+	for _, callback := range callbacks {
 		callback()
 	}
 
@@ -110,8 +124,12 @@ func TestPersistableChannelQueue(t *testing.T) {
 	assert.NoError(t, err)
 
 	go queue.Run(func(shutdown func()) {
+		lock.Lock()
+		defer lock.Unlock()
 		queueShutdown = append(queueShutdown, shutdown)
 	}, func(terminate func()) {
+		lock.Lock()
+		defer lock.Unlock()
 		queueTerminate = append(queueTerminate, terminate)
 	})
 
@@ -122,10 +140,19 @@ func TestPersistableChannelQueue(t *testing.T) {
 	result4 := <-handleChan
 	assert.Equal(t, test2.TestString, result4.TestString)
 	assert.Equal(t, test2.TestInt, result4.TestInt)
-	for _, callback := range queueShutdown {
+
+	lock.Lock()
+	callbacks = make([]func(), len(queueShutdown))
+	copy(callbacks, queueShutdown)
+	lock.Unlock()
+	for _, callback := range callbacks {
 		callback()
 	}
-	for _, callback := range queueTerminate {
+	lock.Lock()
+	callbacks = make([]func(), len(queueTerminate))
+	copy(callbacks, queueTerminate)
+	lock.Unlock()
+	for _, callback := range callbacks {
 		callback()
 	}