Data race fixes in tests and a new semgrep rule (#14594)

* test: don't use loop vars in goroutines

fixes a data race in the test

* test: copy objects in statestore before mutating

fixes data race in test

* test: @lgfa29's segmgrep rule for loops/goroutines

Found 2 places where we were improperly using loop variables inside
goroutines.
This commit is contained in:
Michael Schurter 2022-09-15 10:35:08 -07:00 committed by GitHub
parent 6052aba96d
commit bd4b4b8f66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 4 deletions

28
.semgrep/loopclosure.yml Normal file
View File

@ -0,0 +1,28 @@
rules:
- id: loopclosure
patterns:
- pattern-inside: |
for $A, $B := range $C {
...
}
- pattern-inside: |
go func() {
...
}()
- pattern-not-inside: |
go func(..., $B, ...) {
...
}(..., $B, ...)
- pattern-not-inside: |
go func() {
...
for ... {
...
}
...
}()
- pattern: $B
message: Loop variable $B used inside goroutine
languages:
- go
severity: WARNING

View File

@ -2979,7 +2979,8 @@ func TestDockerDriver_StopSignal(t *testing.T) {
},
}
for _, c := range cases {
for i := range cases {
c := cases[i]
t.Run(c.name, func(t *testing.T) {
taskCfg := newTaskConfig(c.variant, []string{"sleep", "9901"})

View File

@ -228,7 +228,8 @@ func TestMonitor_Monitor_RemoteServer(t *testing.T) {
},
}
for _, tc := range cases {
for i := range cases {
tc := cases[i]
t.Run(tc.desc, func(t *testing.T) {
require := require.New(t)

View File

@ -162,7 +162,7 @@ func TestClientAllocations_GarbageCollectAll_OldNode(t *testing.T) {
// Test for an old version error
node := mock.Node()
node.Attributes["nomad.version"] = "0.7.1"
require.Nil(state.UpsertNode(nstructs.MsgTypeTestSetup, 1005, node))
require.Nil(state.UpsertNode(nstructs.MsgTypeTestSetup, 1005, node.Copy()))
req := &nstructs.NodeSpecificRequest{
NodeID: node.ID,
@ -546,7 +546,7 @@ func TestClientAllocations_Stats_OldNode(t *testing.T) {
// Test for an old version error
node := mock.Node()
node.Attributes["nomad.version"] = "0.7.1"
require.Nil(state.UpsertNode(nstructs.MsgTypeTestSetup, 1005, node))
require.Nil(state.UpsertNode(nstructs.MsgTypeTestSetup, 1005, node.Copy()))
alloc := mock.Alloc()
alloc.NodeID = node.ID