Skip to content

Commit 83c4fac

Browse files
authored
fix(storage): resolve race condition in memory store (#1270)
* fix(storage): resolve race condition in memory store * fix: resolve variable shadowing in CopyEndpointStatus * fix: update test files to use CopyEndpointStatus function
1 parent 37ba305 commit 83c4fac

4 files changed

Lines changed: 38 additions & 26 deletions

File tree

storage/store/memory/memory.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (s *Store) GetAllEndpointStatuses(params *paging.EndpointStatusParams) ([]*
4949
pagedEndpointStatuses := make([]*endpoint.Status, 0, len(allStatuses))
5050
for _, v := range allStatuses {
5151
if status, ok := v.(*endpoint.Status); ok {
52-
pagedEndpointStatuses = append(pagedEndpointStatuses, ShallowCopyEndpointStatus(status, params))
52+
pagedEndpointStatuses = append(pagedEndpointStatuses, CopyEndpointStatus(status, params))
5353
}
5454
}
5555
sort.Slice(pagedEndpointStatuses, func(i, j int) bool {
@@ -87,7 +87,7 @@ func (s *Store) GetEndpointStatusByKey(key string, params *paging.EndpointStatus
8787
if endpointStatus == nil {
8888
return nil, common.ErrEndpointNotFound
8989
}
90-
return ShallowCopyEndpointStatus(endpointStatus.(*endpoint.Status), params), nil
90+
return CopyEndpointStatus(endpointStatus.(*endpoint.Status), params), nil
9191
}
9292

9393
// GetSuiteStatusByKey returns the suite status for a given key

storage/store/memory/util.go

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,35 +6,47 @@ import (
66
"github.com/TwiN/gatus/v5/storage/store/common/paging"
77
)
88

9-
// ShallowCopyEndpointStatus returns a shallow copy of a Status with only the results
10-
// within the range defined by the page and pageSize parameters
11-
func ShallowCopyEndpointStatus(ss *endpoint.Status, params *paging.EndpointStatusParams) *endpoint.Status {
12-
shallowCopy := &endpoint.Status{
9+
// CopyEndpointStatus returns a safe copy of a Status with only the results
10+
// within the range defined by the page and pageSize parameters.
11+
// This function performs deep copying of slices to prevent race conditions
12+
// when the original slice is modified concurrently.
13+
func CopyEndpointStatus(ss *endpoint.Status, params *paging.EndpointStatusParams) *endpoint.Status {
14+
statusCopy := &endpoint.Status{
1315
Name: ss.Name,
1416
Group: ss.Group,
1517
Key: ss.Key,
1618
Uptime: endpoint.NewUptime(),
1719
}
1820
if params == nil || (params.ResultsPage == 0 && params.ResultsPageSize == 0 && params.EventsPage == 0 && params.EventsPageSize == 0) {
19-
shallowCopy.Results = ss.Results
20-
shallowCopy.Events = ss.Events
21+
// Deep copy all results to prevent race conditions
22+
statusCopy.Results = make([]*endpoint.Result, len(ss.Results))
23+
copy(statusCopy.Results, ss.Results)
24+
// Deep copy all events to prevent race conditions
25+
statusCopy.Events = make([]*endpoint.Event, len(ss.Events))
26+
copy(statusCopy.Events, ss.Events)
2127
} else {
2228
numberOfResults := len(ss.Results)
2329
resultsStart, resultsEnd := getStartAndEndIndex(numberOfResults, params.ResultsPage, params.ResultsPageSize)
2430
if resultsStart < 0 || resultsEnd < 0 {
25-
shallowCopy.Results = []*endpoint.Result{}
31+
statusCopy.Results = []*endpoint.Result{}
2632
} else {
27-
shallowCopy.Results = ss.Results[resultsStart:resultsEnd]
33+
// Deep copy the slice range to prevent race conditions
34+
resultRange := ss.Results[resultsStart:resultsEnd]
35+
statusCopy.Results = make([]*endpoint.Result, len(resultRange))
36+
copy(statusCopy.Results, resultRange)
2837
}
2938
numberOfEvents := len(ss.Events)
3039
eventsStart, eventsEnd := getStartAndEndIndex(numberOfEvents, params.EventsPage, params.EventsPageSize)
3140
if eventsStart < 0 || eventsEnd < 0 {
32-
shallowCopy.Events = []*endpoint.Event{}
41+
statusCopy.Events = []*endpoint.Event{}
3342
} else {
34-
shallowCopy.Events = ss.Events[eventsStart:eventsEnd]
43+
// Deep copy the slice range to prevent race conditions
44+
eventRange := ss.Events[eventsStart:eventsEnd]
45+
statusCopy.Events = make([]*endpoint.Event, len(eventRange))
46+
copy(statusCopy.Events, eventRange)
3547
}
3648
}
37-
return shallowCopy
49+
return statusCopy
3850
}
3951

4052
// ShallowCopySuiteStatus returns a shallow copy of a suite Status with only the results

storage/store/memory/util_bench_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ import (
88
"github.com/TwiN/gatus/v5/storage/store/common/paging"
99
)
1010

11-
func BenchmarkShallowCopyEndpointStatus(b *testing.B) {
11+
func BenchmarkCopyEndpointStatus(b *testing.B) {
1212
ep := &testEndpoint
1313
status := endpoint.NewStatus(ep.Group, ep.Name)
1414
for i := 0; i < storage.DefaultMaximumNumberOfResults; i++ {
1515
AddResult(status, &testSuccessfulResult, storage.DefaultMaximumNumberOfResults, storage.DefaultMaximumNumberOfEvents)
1616
}
1717
for n := 0; n < b.N; n++ {
18-
ShallowCopyEndpointStatus(status, paging.NewEndpointStatusParams().WithResults(1, 20))
18+
CopyEndpointStatus(status, paging.NewEndpointStatusParams().WithResults(1, 20))
1919
}
2020
b.ReportAllocs()
2121
}

storage/store/memory/util_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,42 +26,42 @@ func TestAddResult(t *testing.T) {
2626
AddResult(nil, &endpoint.Result{Timestamp: time.Now()}, storage.DefaultMaximumNumberOfResults, storage.DefaultMaximumNumberOfEvents)
2727
}
2828

29-
func TestShallowCopyEndpointStatus(t *testing.T) {
29+
func TestCopyEndpointStatus(t *testing.T) {
3030
ep := &endpoint.Endpoint{Name: "name", Group: "group"}
3131
endpointStatus := endpoint.NewStatus(ep.Group, ep.Name)
3232
ts := time.Now().Add(-25 * time.Hour)
3333
for i := 0; i < 25; i++ {
3434
AddResult(endpointStatus, &endpoint.Result{Success: i%2 == 0, Timestamp: ts}, storage.DefaultMaximumNumberOfResults, storage.DefaultMaximumNumberOfEvents)
3535
ts = ts.Add(time.Hour)
3636
}
37-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(-1, -1)).Results) != 0 {
37+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(-1, -1)).Results) != 0 {
3838
t.Error("expected to have 0 result")
3939
}
40-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 1)).Results) != 1 {
40+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 1)).Results) != 1 {
4141
t.Error("expected to have 1 result")
4242
}
43-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(5, 0)).Results) != 0 {
43+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(5, 0)).Results) != 0 {
4444
t.Error("expected to have 0 results")
4545
}
46-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(-1, 20)).Results) != 0 {
46+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(-1, 20)).Results) != 0 {
4747
t.Error("expected to have 0 result, because the page was invalid")
4848
}
49-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, -1)).Results) != 0 {
49+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, -1)).Results) != 0 {
5050
t.Error("expected to have 0 result, because the page size was invalid")
5151
}
52-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 10)).Results) != 10 {
52+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 10)).Results) != 10 {
5353
t.Error("expected to have 10 results, because given a page size of 10, page 1 should have 10 elements")
5454
}
55-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(2, 10)).Results) != 10 {
55+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(2, 10)).Results) != 10 {
5656
t.Error("expected to have 10 results, because given a page size of 10, page 2 should have 10 elements")
5757
}
58-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(3, 10)).Results) != 5 {
58+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(3, 10)).Results) != 5 {
5959
t.Error("expected to have 5 results, because given a page size of 10, page 3 should have 5 elements")
6060
}
61-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(4, 10)).Results) != 0 {
61+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(4, 10)).Results) != 0 {
6262
t.Error("expected to have 0 results, because given a page size of 10, page 4 should have 0 elements")
6363
}
64-
if len(ShallowCopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 50)).Results) != 25 {
64+
if len(CopyEndpointStatus(endpointStatus, paging.NewEndpointStatusParams().WithResults(1, 50)).Results) != 25 {
6565
t.Error("expected to have 25 results, because there's only 25 results")
6666
}
6767
}

0 commit comments

Comments
 (0)