fix: found races in accessing globalLocalDrives (minio#19069)

harshavardhana · web-flow · commit b6e98aed0161 · 2024-02-16T17:15:57.000-08:00
make a copy before accessing globalLocalDrives

Bonus: update console v0.46.0

Signed-off-by: Harshavardhana &lt;harsha@minio.io&gt;
diff --git a/.github/workflows/multipart/migrate.sh b/.github/workflows/multipart/migrate.sh
@@ -25,8 +25,8 @@ if [ ! -f ./mc ]; then
 fi
 
 (
-	cd /tmp
-	go install github.com/minio/minio/docs/debugging/s3-check-md5@master
+	cd ./docs/debugging/s3-check-md5
+	go install -v
 )
 
 export RELEASE=RELEASE.2023-08-29T23-07-35Z
diff --git a/Makefile b/Makefile
@@ -177,9 +177,9 @@ docker: build ## builds minio docker container
 	@docker build -q --no-cache -t $(TAG) . -f Dockerfile
 
 install-race: checks ## builds minio to $(PWD)
-	@echo "Building minio binary to './minio'"
+	@echo "Building minio binary with -race to './minio'"
 	@GORACE=history_size=7 CGO_ENABLED=1 go build -tags kqueue -race -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null
-	@echo "Installing minio binary to '$(GOPATH)/bin/minio'"
+	@echo "Installing minio binary with -race to '$(GOPATH)/bin/minio'"
 	@mkdir -p $(GOPATH)/bin && cp -f $(PWD)/minio $(GOPATH)/bin/minio
 
 install: build ## builds minio and installs it to $GOPATH/bin.
diff --git a/buildscripts/rewrite-old-new.sh b/buildscripts/rewrite-old-new.sh
@@ -87,7 +87,11 @@ function verify_rewrite() {
 		exit 1
 	fi
 
-	go install github.com/minio/minio/docs/debugging/s3-check-md5@master
+	(
+		cd ./docs/debugging/s3-check-md5
+		go install -v
+	)
+
 	if ! s3-check-md5 \
 		-debug \
 		-versions \
diff --git a/cmd/background-newdisks-heal-ops.go b/cmd/background-newdisks-heal-ops.go
@@ -353,7 +353,7 @@ func initAutoHeal(ctx context.Context, objAPI ObjectLayer) {
 
 func getLocalDisksToHeal() (disksToHeal Endpoints) {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 	for _, disk := range localDrives {
 		_, err := disk.GetDiskID()
diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go
@@ -3393,7 +3393,7 @@ func (p *ReplicationPool) persistToDrive(ctx context.Context, v MRFReplicateEntr
 	}
 
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	for _, localDrive := range localDrives {
@@ -3460,7 +3460,7 @@ func (p *ReplicationPool) loadMRF() (mrfRec MRFReplicateEntries, err error) {
 	}
 
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	for _, localDrive := range localDrives {
diff --git a/cmd/metrics-resource.go b/cmd/metrics-resource.go
@@ -274,7 +274,7 @@ func collectDriveMetrics(m madmin.RealtimeMetrics) {
 	}
 
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	for _, d := range localDrives {
diff --git a/cmd/peer-rest-server.go b/cmd/peer-rest-server.go
@@ -786,7 +786,7 @@ var errUnsupportedSignal = fmt.Errorf("unsupported signal")
 
 func waitingDrivesNode() map[string]madmin.DiskMetrics {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	errs := make([]error, len(localDrives))
diff --git a/cmd/peer-s3-server.go b/cmd/peer-s3-server.go
@@ -82,7 +82,7 @@ func (s *peerS3Server) HealthHandler(w http.ResponseWriter, r *http.Request) {
 
 func healBucketLocal(ctx context.Context, bucket string, opts madmin.HealOpts) (res madmin.HealResultItem, err error) {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	// Initialize sync waitgroup.
@@ -206,7 +206,7 @@ func healBucketLocal(ctx context.Context, bucket string, opts madmin.HealOpts) (
 
 func listBucketsLocal(ctx context.Context, opts BucketOptions) (buckets []BucketInfo, err error) {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	quorum := (len(localDrives) / 2)
@@ -252,9 +252,15 @@ func listBucketsLocal(ctx context.Context, opts BucketOptions) (buckets []Bucket
 	return buckets, nil
 }
 
+func cloneDrives(drives []StorageAPI) []StorageAPI {
+	newDrives := make([]StorageAPI, len(drives))
+	copy(newDrives, drives)
+	return newDrives
+}
+
 func getBucketInfoLocal(ctx context.Context, bucket string, opts BucketOptions) (BucketInfo, error) {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	g := errgroup.WithNErrs(len(localDrives)).WithConcurrency(32)
@@ -303,7 +309,7 @@ func getBucketInfoLocal(ctx context.Context, bucket string, opts BucketOptions)
 
 func deleteBucketLocal(ctx context.Context, bucket string, opts DeleteBucketOptions) error {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	g := errgroup.WithNErrs(len(localDrives)).WithConcurrency(32)
@@ -341,7 +347,7 @@ func deleteBucketLocal(ctx context.Context, bucket string, opts DeleteBucketOpti
 
 func makeBucketLocal(ctx context.Context, bucket string, opts MakeBucketOptions) error {
 	globalLocalDrivesMu.RLock()
-	localDrives := globalLocalDrives
+	localDrives := cloneDrives(globalLocalDrives)
 	globalLocalDrivesMu.RUnlock()
 
 	g := errgroup.WithNErrs(len(localDrives)).WithConcurrency(32)
diff --git a/docs/bucket/replication/delete-replication.sh b/docs/bucket/replication/delete-replication.sh
@@ -75,9 +75,13 @@ echo "=== myminio1"
 echo "=== myminio2"
 ./mc ls --versions myminio2/testbucket/dir/file
 
-versionId="$(mc ls --json --versions myminio1/testbucket/dir/ | tail -n1 | jq -r .versionId)"
+versionId="$(./mc ls --json --versions myminio1/testbucket/dir/ | tail -n1 | jq -r .versionId)"
 
-aws s3api --endpoint-url http://localhost:9001 --profile minio delete-object --bucket testbucket --key dir/file --version-id "$versionId"
+aws configure set aws_access_key_id minioadmin --profile minioadmin
+aws configure set aws_secret_access_key minioadmin --profile minioadmin
+aws configure set default.region us-east-1 --profile minioadmin
+
+aws s3api --endpoint-url http://localhost:9001 --profile minioadmin delete-object --bucket testbucket --key dir/file --version-id "$versionId"
 
 ./mc ls -r --versions myminio1/testbucket >/tmp/myminio1.txt
 ./mc ls -r --versions myminio2/testbucket >/tmp/myminio2.txt
diff --git a/docs/bucket/replication/setup_3site_replication.sh b/docs/bucket/replication/setup_3site_replication.sh
@@ -41,7 +41,10 @@ unset MINIO_KMS_KES_KEY_FILE
 unset MINIO_KMS_KES_ENDPOINT
 unset MINIO_KMS_KES_KEY_NAME
 
-go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
+(
+	cd ./docs/debugging/s3-check-md5
+	go install -v
+)
 
 wget -O mc https://dl.minio.io/client/mc/release/linux-amd64/mc &&
 	chmod +x mc
diff --git a/docs/debugging/s3-check-md5/main.go b/docs/debugging/s3-check-md5/main.go
@@ -163,7 +163,7 @@ func main() {
 				continue
 			}
 			if v, ok := object.UserMetadata["X-Amz-Server-Side-Encryption"]; ok && v == "aws:kms" {
-				log.Println("FAILED: encrypted with SSE-KMS do not have md5sum as ETag:", objFullPath(object))
+				log.Println("SKIPPED: encrypted with SSE-KMS do not have md5sum as ETag:", objFullPath(object))
 				continue
 			}
 			parts := 1
diff --git a/docs/distributed/decom-compressed-sse-s3.sh b/docs/distributed/decom-compressed-sse-s3.sh
@@ -147,7 +147,10 @@ if [ $ret -ne 0 ]; then
 	exit 1
 fi
 
-go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
+(
+	cd ./docs/debugging/s3-check-md5
+	go install -v
+)
 
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
diff --git a/docs/distributed/decom-encrypted-sse-s3.sh b/docs/distributed/decom-encrypted-sse-s3.sh
@@ -158,7 +158,10 @@ if [ $ret -ne 0 ]; then
 	exit 1
 fi
 
-go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
+(
+	cd ./docs/debugging/s3-check-md5
+	go install -v
+)
 
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
diff --git a/docs/distributed/decom-encrypted.sh b/docs/distributed/decom-encrypted.sh
@@ -144,7 +144,10 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
 	exit 1
 fi
 
-go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
+(
+	cd ./docs/debugging/s3-check-md5
+	go install -v
+)
 
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
diff --git a/docs/distributed/decom.sh b/docs/distributed/decom.sh
@@ -210,7 +210,10 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
 	exit 1
 fi
 
-go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
+(
+	cd ./docs/debugging/s3-check-md5
+	go install -v
+)
 
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
 s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
diff --git a/go.mod b/go.mod
@@ -45,7 +45,7 @@ require (
 	github.com/lithammer/shortuuid/v4 v4.0.0
 	github.com/miekg/dns v1.1.58
 	github.com/minio/cli v1.24.2
-	github.com/minio/console v0.45.1-0.20240213061721-ee4d7b9b6940
+	github.com/minio/console v0.46.0
 	github.com/minio/csvparser v1.0.0
 	github.com/minio/dnscache v0.1.1
 	github.com/minio/dperf v0.5.3
diff --git a/go.sum b/go.sum
@@ -432,8 +432,8 @@ github.com/minio/cli v1.24.2 h1:J+fCUh9mhPLjN3Lj/YhklXvxj8mnyE/D6FpFduXJ2jg=
 github.com/minio/cli v1.24.2/go.mod h1:bYxnK0uS629N3Bq+AOZZ+6lwF77Sodk4+UL9vNuXhOY=
 github.com/minio/colorjson v1.0.6 h1:m7TUvpvt0u7FBmVIEQNIa0T4NBQlxrcMBp4wJKsg2Ik=
 github.com/minio/colorjson v1.0.6/go.mod h1:LUXwS5ZGNb6Eh9f+t+3uJiowD3XsIWtsvTriUBeqgYs=
-github.com/minio/console v0.45.1-0.20240213061721-ee4d7b9b6940 h1:DdXAWKUmiSMFS3pPG351kwYo2YlC0EaKKCNStAmd7xo=
-github.com/minio/console v0.45.1-0.20240213061721-ee4d7b9b6940/go.mod h1:VvJdNfELVCc2VELF1rJtIPCb3FcWGtNfM/Ktvnu2MZ0=
+github.com/minio/console v0.46.0 h1:So7y3csQl7m3Llaxmbg5xiTCwlElUol8kUyVOViBRFY=
+github.com/minio/console v0.46.0/go.mod h1:VvJdNfELVCc2VELF1rJtIPCb3FcWGtNfM/Ktvnu2MZ0=
 github.com/minio/csvparser v1.0.0 h1:xJEHcYK8ZAjeW4hNV9Zu30u+/2o4UyPnYgyjWp8b7ZU=
 github.com/minio/csvparser v1.0.0/go.mod h1:lKXskSLzPgC5WQyzP7maKH7Sl1cqvANXo9YCto8zbtM=
 github.com/minio/dnscache v0.1.1 h1:AMYLqomzskpORiUA1ciN9k7bZT1oB3YZN4cEIi88W5o=

Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,8 @@ if [ ! -f ./mc ]; then`
`25`	`25`	`fi`
`26`	`26`
`27`	`27`	`(`
`28`		`- cd /tmp`
`29`		`- go install github.com/minio/minio/docs/debugging/s3-check-md5@master`
	`28`	`+ cd ./docs/debugging/s3-check-md5`
	`29`	`+ go install -v`
`30`	`30`	`)`
`31`	`31`
`32`	`32`	`export RELEASE=RELEASE.2023-08-29T23-07-35Z`
Original file line number	Diff line number	Diff line change
`@@ -3393,7 +3393,7 @@ func (p *ReplicationPool) persistToDrive(ctx context.Context, v MRFReplicateEntr`
`3393`	`3393`	`}`
`3394`	`3394`
`3395`	`3395`	`globalLocalDrivesMu.RLock()`
`3396`		`- localDrives := globalLocalDrives`
	`3396`	`+ localDrives := cloneDrives(globalLocalDrives)`
`3397`	`3397`	`globalLocalDrivesMu.RUnlock()`
`3398`	`3398`
`3399`	`3399`	`for _, localDrive := range localDrives {`
`@@ -3460,7 +3460,7 @@ func (p *ReplicationPool) loadMRF() (mrfRec MRFReplicateEntries, err error) {`
`3460`	`3460`	`}`
`3461`	`3461`
`3462`	`3462`	`globalLocalDrivesMu.RLock()`
`3463`		`- localDrives := globalLocalDrives`
	`3463`	`+ localDrives := cloneDrives(globalLocalDrives)`
`3464`	`3464`	`globalLocalDrivesMu.RUnlock()`
`3465`	`3465`
`3466`	`3466`	`for _, localDrive := range localDrives {`
Original file line number	Diff line number	Diff line change
`@@ -274,7 +274,7 @@ func collectDriveMetrics(m madmin.RealtimeMetrics) {`
`274`	`274`	`}`
`275`	`275`
`276`	`276`	`globalLocalDrivesMu.RLock()`
`277`		`- localDrives := globalLocalDrives`
	`277`	`+ localDrives := cloneDrives(globalLocalDrives)`
`278`	`278`	`globalLocalDrivesMu.RUnlock()`
`279`	`279`
`280`	`280`	`for _, d := range localDrives {`