Skip to content

Commit fab1af6

Browse files
authored
fix(bbr): add read lock to GetBaseModel to prevent concurrent map crash (#2570)
- `GetBaseModel()` reads `loraAdapterToBaseModel` and `baseModels` maps without holding a lock, while `ConfigMapUpdateOrAddIfNotExist()` and `ConfigMapDelete()` write to these maps under `ds.lock.Lock()`. In Go, concurrent map read and write causes `fatal error: concurrent map read and map write`, which crashes the process and cannot be recovered. - Add `ds.lock.RLock()`/`RUnlock()` to `GetBaseModel()` to ensure safe concurrent access. - Add a concurrent read/write test to prevent regression. - go test -race ./pkg/bbr/datastore/... test passed.
1 parent 4d97138 commit fab1af6

2 files changed

Lines changed: 17 additions & 0 deletions

File tree

pkg/bbr/datastore/datastore.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ func (ds *datastore) ConfigMapDelete(configmap *corev1.ConfigMap) {
128128

129129
func (ds *datastore) GetBaseModel(modelName string) string {
130130
trimmedModelName := strings.TrimSpace(modelName)
131+
ds.lock.RLock()
132+
defer ds.lock.RUnlock()
131133
// if the given model name is a LoRA adapter, we should return its base model
132134
if baseModel, ok := ds.loraAdapterToBaseModel[trimmedModelName]; ok {
133135
return baseModel

pkg/bbr/datastore/datastore_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,21 @@ func TestConfigMapDelete_MissingBaseModel(t *testing.T) {
191191
}
192192
}
193193

194+
func TestGetBaseModel_ConcurrentRace(t *testing.T) {
195+
ds := NewDatastore()
196+
cm := makeConfigMap(cmName, baseModel, "- a1\n")
197+
198+
go func() {
199+
for i := 0; i < 10000; i++ {
200+
_ = ds.ConfigMapUpdateOrAddIfNotExist(cm)
201+
}
202+
}()
203+
204+
for i := 0; i < 10000; i++ {
205+
ds.GetBaseModel("a1")
206+
}
207+
}
208+
194209
func TestConfigMapDelete_BaseModelCount(t *testing.T) {
195210
ds := NewDatastore()
196211
cm1 := makeConfigMap(cmName, baseModel, "- a1\n")

0 commit comments

Comments
 (0)