weed server: startup can die with `concurrent map read and map write` because `ViperProxy` does not lock `GetStringMap`
weed server can die during startup with fatal error: concurrent map read and map write. This one is from ClickHouse CI on 4.42 (full log):
fatal error: concurrent map read and map write
goroutine 70 [running]:
internal/runtime/maps.fatal({0x54ac7d0?, 0x0?})
/usr/local/go/src/runtime/panic.go:1181
github.com/spf13/viper.(*Viper).searchMap(0x148c31764300, 0x148c3212aaa0?, {0x148c3212aaa0, 0x2, 0x2})
/go/pkg/mod/github.com/spf13/[email protected]/viper.go:483
github.com/spf13/viper.(*Viper).find(0x148c31764300, {0x5436c9c, 0xf}, 0x1)
/go/pkg/mod/github.com/spf13/[email protected]/viper.go:1269
github.com/spf13/viper.(*Viper).Get(0x148c31764300, {0x5436c9c?, 0x93cf150?})
/go/pkg/mod/github.com/spf13/[email protected]/viper.go:718
github.com/spf13/viper.(*Viper).GetStringMap(0x0?, {0x5436c9c?, 0x58b7c80?})
/go/pkg/mod/github.com/spf13/[email protected]/viper.go:898
github.com/seaweedfs/seaweedfs/weed/storage/backend.LoadConfiguration(0x9481770)
/github/workspace/weed/storage/backend/backend.go:52
github.com/seaweedfs/seaweedfs/weed/command.startMaster({...}, ...)
/github/workspace/weed/command/master.go:183
created by github.com/seaweedfs/seaweedfs/weed/command.runServer in goroutine 1
/github/workspace/weed/command/server.go:451It is still present on current master (4914c149), which the line numbers below refer to: weed/util/config.go is byte-identical from 4.42 to master, and viper is still v1.21.0.
ViperProxy (weed/util/config.go:75) guards the global viper with a mutex, but only in the five methods it wraps: SetDefault, GetString, GetBool, GetInt, GetStringSlice. GetStringMap is promoted from the embedded *viper.Viper and takes no lock. runServer starts the volume server and the master back to back (weed/command/server.go:456 and :460, without the 1-2 s delay the other services get), and then:
NewVolumeServerwrites viper's defaults map throughv.SetDefault("jwt.signing.expires_after_seconds", 10)(weed/server/volume_server.go:91), holding the mutex;startMasterreads the same map throughconfig.GetStringMap("storage.backend")inbackend.LoadConfiguration(weed/storage/backend/backend.go:53, called fromweed/command/master.go:187), without it.
This is the read-side sibling of #1272 (concurrent SetDefault calls during weed server startup): ViperProxy now serializes SetDefault, but not this read.
Reproducer
A race-enabled build of master (go1.26.8, linux/amd64) reports exactly this pair on a plain start:
go build -race -o weed-race ./weed
./weed-race server -ip=127.0.0.1 -ip.bind=127.0.0.1 -dir="$(mktemp -d)" \
-master.port=21450 -master.port.grpc=21451 -volume.port=21460 -volume.port.grpc=21461WARNING: DATA RACE
Write at 0x00c000d00540 by goroutine 518:
runtime.mapassign_faststr()
github.com/spf13/viper.deepSearch()
github.com/spf13/[email protected]/util.go:196
github.com/spf13/viper.(*Viper).SetDefault()
github.com/spf13/[email protected]/viper.go:1489
github.com/seaweedfs/seaweedfs/weed/util.(*ViperProxy).SetDefault()
weed/util/config.go:87
github.com/seaweedfs/seaweedfs/weed/server.NewVolumeServer()
weed/server/volume_server.go:91
github.com/seaweedfs/seaweedfs/weed/command.VolumeServerOptions.startVolumeServer()
weed/command/volume.go:440
github.com/seaweedfs/seaweedfs/weed/command.runServer.gowrap3()
weed/command/server.go:456
Previous read at 0x00c000d00540 by goroutine 519:
runtime.mapaccess2_faststr()
github.com/spf13/viper.(*Viper).searchMap()
github.com/spf13/[email protected]/viper.go:483
github.com/spf13/viper.(*Viper).find()
github.com/spf13/[email protected]/viper.go:1269
github.com/spf13/viper.(*Viper).Get()
github.com/spf13/[email protected]/viper.go:718
github.com/spf13/viper.(*Viper).GetStringMap()
github.com/spf13/[email protected]/viper.go:898
github.com/seaweedfs/seaweedfs/weed/storage/backend.LoadConfiguration()
weed/storage/backend/backend.go:53
github.com/seaweedfs/seaweedfs/weed/command.startMaster()
weed/command/master.go:187
github.com/seaweedfs/seaweedfs/weed/command.runServer.gowrap4()
weed/command/server.go:460The race detector only reports it when no mutex hand-off happens to order the two accesses, so it fired on 1 of 12 starts. This test in weed/util calls the same two methods from two goroutines and reproduces it reliably:
func TestViperProxyGetStringMapRace(t *testing.T) {
v := GetViper()
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
for i := 0; i < 200000; i++ {
v.SetDefault(fmt.Sprintf("repro%d.expires_after_seconds", i), 10)
}
}()
for i := 0; i < 200000; i++ {
_ = v.GetStringMap("storage.backend")
}
wg.Wait()
}Without -race it dies with fatal error: concurrent map read and map write in 10 of 10 runs, 8 of them through the same viper frames as the CI crash (viper.go:483, :1269, :718, :898). With -race it reports the race in 3 of 3 runs.
Possible fix
Wrap GetStringMap like the other five:
func (vp *ViperProxy) GetStringMap(key string) map[string]interface{} {
vp.Lock()
defer vp.Unlock()
return vp.Viper.GetStringMap(key)
}With only this change the test passes 10 of 10 runs and 3 of 3 under -race, and 12 starts of the race-enabled weed server report no race.
Any other method reached through the embedding bypasses the mutex the same way, for example config.AllKeys() in weed/filer/configuration.go:50. Holding *viper.Viper in a named field instead of embedding it would turn every unwrapped call into a compile error. Package-level util.LoadConfiguration (weed/util/config.go:41) also writes the global viper without the mutex. On weed server it runs before the services start, but the Reload() handlers call it later (weed/server/volume_server.go:190, weed/server/master_server.go:652).
Found in ClickHouse CI, which starts weed server for its stateless tests; one such crash leaves a whole test job without results. ClickHouse/ClickHouse#122320 makes that setup fail fast when weed dies.
Source: seaweedfs/seaweedfs