Skip to content

Commit 81adae1

Browse files
agneumNikolayS
authored andcommitted
fix(engine): avoid panic with concurrent map read and map write (#503)
1 parent 3d17e33 commit 81adae1

File tree

2 files changed

+64
-5
lines changed

2 files changed

+64
-5
lines changed

engine/internal/cloning/snapshots.go

+20-5
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,13 @@ func (c *Base) fetchSnapshots() error {
3030
var latestSnapshot *models.Snapshot
3131

3232
snapshots := make(map[string]*models.Snapshot, len(entries))
33+
cloneCounter := c.cloneCounter()
3334

3435
for _, entry := range entries {
3536
numClones := 0
3637

37-
for cloneName := range c.clones {
38-
if c.clones[cloneName] != nil && c.clones[cloneName].Clone.Snapshot != nil &&
39-
c.clones[cloneName].Clone.Snapshot.ID == entry.ID {
40-
numClones++
41-
}
38+
if num, ok := cloneCounter[entry.ID]; ok {
39+
numClones = num
4240
}
4341

4442
currentSnapshot := &models.Snapshot{
@@ -61,6 +59,23 @@ func (c *Base) fetchSnapshots() error {
6159

6260
return nil
6361
}
62+
63+
func (c *Base) cloneCounter() map[string]int {
64+
cloneCounter := make(map[string]int)
65+
66+
c.cloneMutex.RLock()
67+
68+
for cloneName := range c.clones {
69+
if c.clones[cloneName] != nil && c.clones[cloneName].Clone.Snapshot != nil {
70+
cloneCounter[c.clones[cloneName].Clone.Snapshot.ID]++
71+
}
72+
}
73+
74+
c.cloneMutex.RUnlock()
75+
76+
return cloneCounter
77+
}
78+
6479
func (c *Base) resetSnapshots(snapshotMap map[string]*models.Snapshot, latestSnapshot *models.Snapshot) {
6580
c.snapshotBox.snapshotMutex.Lock()
6681

engine/internal/cloning/snapshots_test.go

+44
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,50 @@ func TestCloneCounter(t *testing.T) {
121121
require.Equal(t, 0, snapshot.NumClones)
122122
}
123123

124+
func TestInitialCloneCounter(t *testing.T) {
125+
c := &Base{}
126+
c.clones = make(map[string]*CloneWrapper)
127+
128+
snapshot := &models.Snapshot{
129+
ID: "testSnapshotID",
130+
}
131+
132+
snapshot2 := &models.Snapshot{
133+
ID: "testSnapshotID2",
134+
}
135+
136+
cloneWrapper01 := &CloneWrapper{
137+
Clone: &models.Clone{
138+
ID: "test_clone001",
139+
Snapshot: snapshot,
140+
},
141+
}
142+
143+
cloneWrapper02 := &CloneWrapper{
144+
Clone: &models.Clone{
145+
ID: "test_clone002",
146+
Snapshot: snapshot,
147+
},
148+
}
149+
150+
cloneWrapper03 := &CloneWrapper{
151+
Clone: &models.Clone{
152+
ID: "test_clone003",
153+
Snapshot: snapshot2,
154+
},
155+
}
156+
157+
c.clones["test_clone001"] = cloneWrapper01
158+
c.clones["test_clone002"] = cloneWrapper02
159+
c.clones["test_clone003"] = cloneWrapper03
160+
161+
counters := c.cloneCounter()
162+
163+
require.Equal(t, 2, len(counters))
164+
require.Equal(t, 2, counters["testSnapshotID"])
165+
require.Equal(t, 1, counters["testSnapshotID2"])
166+
}
167+
124168
func TestLatestSnapshots(t *testing.T) {
125169
baseSnapshot := &models.Snapshot{
126170
DataStateAt: &models.LocalTime{Time: time.Date(2020, 02, 19, 0, 0, 0, 0, time.UTC)},

0 commit comments

Comments
 (0)