topology: fix fatal concurrent map read/write on VolumeLayout.crowded (#11216)

SetVolumeCrowded mutated the crowded map under accessLock.RLock(), while
GetWritableVolumeCount reads the same map under RLock() on the Assign hot
path. Two concurrent RLock holders with one writing and one reading the
map triggers a fatal "concurrent map read and map write" that kills the
master process (unrecoverable, bypasses recover).

Take the write lock in SetVolumeCrowded instead. This event path is a
low-frequency single consumer driven by the crowded-volume event loop,
and every other mutation of crowded already holds Lock(); setVolumeCrowded
takes no nested locks, so there is no deadlock path. The hot readers
(GetWritableVolumeCount, CloneWritableVolumes) keep using RLock.

Adds a -race regression test that fails (race detected) on the old RLock
and passes with the write lock.

Fixes #11211

weed/topology/crowded_writable_test.go

@@ -1,6 +1,7 @@
 package topology
 
 import (
+	"sync"
 	"testing"
 	"time"
 
@@ -73,3 +74,52 @@ func TestCrowdedCountStillMatchesWritables(t *testing.T) {
 		t.Errorf("writable=%d crowded=%d, want 1 and 1", writable, crowded)
 	}
 }
+
+// SetVolumeCrowded mutates the crowded map while GetWritableVolumeCount reads
+// it on the Assign hot path. Under RLock both could run at once and the Go
+// runtime aborts with "concurrent map read and map write". Run with -race to
+// reproduce the original bug; the write lock makes this pass.
+func TestSetVolumeCrowdedNoRaceWithGetWritableVolumeCount(t *testing.T) {
+	rp, _ := super_block.NewReplicaPlacementFromString("000")
+	vl := NewVolumeLayout(rp, needle.EMPTY_TTL, types.HardDriveType, 10000, false)
+	// GetWritableVolumeCount iterates vl.writables, so give it something to
+	// read while the crowded map is being mutated.
+	vl.writables = []needle.VolumeId{1, 2, 3}
+
+	var wg sync.WaitGroup
+	stop := make(chan struct{})
+	for range 8 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for {
+				select {
+				case <-stop:
+					return
+				default:
+					vl.SetVolumeCrowded(needle.VolumeId(2))
+				}
+			}
+		}()
+	}
+	for range 8 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for {
+				select {
+				case <-stop:
+					return
+				default:
+					vl.GetWritableVolumeCount()
+				}
+			}
+		}()
+	}
+
+	// The race detector needs only a brief window to flag a concurrent
+	// map read/write; let the goroutines hammer the lock briefly.
+	time.Sleep(50 * time.Millisecond)
+	close(stop)
+	wg.Wait()
+}

weed/topology/volume_layout.go

@@ -1066,11 +1066,14 @@ func (vl *VolumeLayout) setVolumeCrowded(vid needle.VolumeId) {
 }
 
 func (vl *VolumeLayout) SetVolumeCrowded(vid needle.VolumeId) {
-	// since delete is guarded by accessLock.Lock(),
-	// and is always called in sequential order,
-	// RLock() should be safe enough
-	vl.accessLock.RLock()
-	defer vl.accessLock.RUnlock()
+	// setVolumeCrowded mutates the crowded map, and GetWritableVolumeCount
+	// reads it under RLock on the Assign hot path. Two concurrent RLock
+	// holders with one writing the map triggers a fatal
+	// "concurrent map read and map write". Take the write lock: this path
+	// is a low-frequency single consumer driven by the crowded-volume
+	// event loop, and every other mutation of crowded already holds Lock().
+	vl.accessLock.Lock()
+	defer vl.accessLock.Unlock()
 
 	vl.setVolumeCrowded(vid)
 }