Skip to content

Commit 117138b

Browse files
🐛 Make sure that an improperly initialised cancel store doesn't cause a panic (#657)
<!-- Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors. All rights reserved. SPDX-License-Identifier: Apache-2.0 --> ### Description <!-- Please add any detail or context that would be useful to a reviewer. --> Make sure that an improperly initialised cancel store doesn't cause a panic ### Test Coverage <!-- Please put an `x` in the correct box e.g. `[x]` to indicate the testing coverage of this change. --> - [x] This change is covered by existing or additional automated tests. - [ ] Manual testing has been performed (and evidence provided) as automated testing was not feasible. - [ ] Additional tests are not required for this change (e.g. documentation update).
1 parent d9a67ca commit 117138b

File tree

3 files changed

+52
-15
lines changed

3 files changed

+52
-15
lines changed

changes/20250729105643.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:bug: Make sure that an improperly initialised cancel store doesn't cause a panic

utils/parallelisation/cancel_functions.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import (
1010

1111
"github.com/sasha-s/go-deadlock"
1212
"golang.org/x/sync/errgroup"
13+
14+
"github.com/ARM-software/golang-utils/utils/commonerrors"
15+
"github.com/ARM-software/golang-utils/utils/reflection"
1316
)
1417

1518
func newFunctionStore[T any](clearOnExecution, stopOnFirstError bool, executeFunc func(context.Context, T) error) *store[T] {
@@ -45,6 +48,9 @@ func (s *store[T]) Len() int {
4548
func (s *store[T]) Execute(ctx context.Context) error {
4649
defer s.mu.Unlock()
4750
s.mu.Lock()
51+
if reflection.IsEmpty(s.executeFunc) {
52+
return commonerrors.New(commonerrors.ErrUndefined, "the cancel store was not initialised correctly")
53+
}
4854
g, gCtx := errgroup.WithContext(ctx)
4955
if !s.stopOnFirstError {
5056
gCtx = ctx
@@ -75,6 +81,7 @@ func (s *CancelFunctionStore) RegisterCancelFunction(cancel ...context.CancelFun
7581
s.store.RegisterFunction(cancel...)
7682
}
7783

84+
// Cancel will execute the cancel functions in the store. Any errors will be ignored and Execute() is recommended if you need to know if a cancellation failed
7885
func (s *CancelFunctionStore) Cancel() {
7986
_ = s.Execute(context.Background())
8087
}

utils/parallelisation/cancel_functions_test.go

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,62 @@
55
package parallelisation
66

77
import (
8+
"context"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
12+
13+
"github.com/ARM-software/golang-utils/utils/commonerrors"
14+
"github.com/ARM-software/golang-utils/utils/commonerrors/errortest"
1115
)
1216

1317
// Given a CancelFunctionsStore
1418
// Functions can be registered
1519
// and all functions will be called
1620
func TestCancelFunctionStore(t *testing.T) {
17-
// Set up some fake CancelFuncs to make sure they are called
18-
called1 := false
19-
called2 := false
20-
cancelFunc1 := func() {
21-
called1 = true
22-
}
23-
cancelFunc2 := func() {
24-
called2 = true
25-
}
21+
t.Run("valid cancel store", func(t *testing.T) {
22+
// Set up some fake CancelFuncs to make sure they are called
23+
called1 := false
24+
called2 := false
25+
cancelFunc1 := func() {
26+
called1 = true
27+
}
28+
cancelFunc2 := func() {
29+
called2 = true
30+
}
31+
32+
store := NewCancelFunctionsStore()
33+
34+
store.RegisterCancelFunction(cancelFunc1, cancelFunc2)
35+
36+
assert.Equal(t, 2, store.Len())
37+
38+
store.Cancel()
39+
40+
assert.True(t, called1)
41+
assert.True(t, called2)
42+
})
43+
44+
t.Run("incorrectly initialised cancel store", func(t *testing.T) {
45+
called1 := false
46+
called2 := false
47+
cancelFunc1 := func() {
48+
called1 = true
49+
}
50+
cancelFunc2 := func() {
51+
called2 = true
52+
}
2653

27-
store := NewCancelFunctionsStore()
54+
store := CancelFunctionStore{}
2855

29-
store.RegisterCancelFunction(cancelFunc1, cancelFunc2)
56+
store.RegisterCancelFunction(cancelFunc1, cancelFunc2)
3057

31-
assert.Equal(t, 2, store.Len())
58+
assert.Equal(t, 2, store.Len())
3259

33-
store.Cancel()
60+
err := store.Execute(context.Background())
61+
errortest.AssertError(t, err, commonerrors.ErrUndefined)
3462

35-
assert.True(t, called1)
36-
assert.True(t, called2)
63+
assert.False(t, called1)
64+
assert.False(t, called2)
65+
})
3766
}

0 commit comments

Comments
 (0)