mirror of https://github.com/status-im/consul.git
clone the service under lock to avoid a data race (#11940)
* clone the service under lock to avoid a data race * add change log * create a struct and copy the pointer to mutate it to avoid a data race * fix failing test * revert added space * add comments, to clarify the data race.
This commit is contained in:
parent
d0274d38a9
commit
1eac39ae9c
|
@ -0,0 +1,3 @@
|
||||||
|
```release-note:bug
|
||||||
|
Mutate `NodeService` struct properly to avoid a data race.
|
||||||
|
```
|
|
@ -1082,27 +1082,34 @@ func (l *State) updateSyncState() error {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// to avoid a data race with the service struct,
|
||||||
|
// We copy the Service struct, mutate it and replace the pointer
|
||||||
|
svc := *ls.Service
|
||||||
|
|
||||||
// If our definition is different, we need to update it. Make a
|
// If our definition is different, we need to update it. Make a
|
||||||
// copy so that we don't retain a pointer to any actual state
|
// copy so that we don't retain a pointer to any actual state
|
||||||
// store info for in-memory RPCs.
|
// store info for in-memory RPCs.
|
||||||
if ls.Service.EnableTagOverride {
|
if svc.EnableTagOverride {
|
||||||
ls.Service.Tags = make([]string, len(rs.Tags))
|
svc.Tags = make([]string, len(rs.Tags))
|
||||||
copy(ls.Service.Tags, rs.Tags)
|
copy(svc.Tags, rs.Tags)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merge any tagged addresses with the consul- prefix (set by the server)
|
// Merge any tagged addresses with the consul- prefix (set by the server)
|
||||||
// back into the local state.
|
// back into the local state.
|
||||||
if !reflect.DeepEqual(ls.Service.TaggedAddresses, rs.TaggedAddresses) {
|
if !reflect.DeepEqual(svc.TaggedAddresses, rs.TaggedAddresses) {
|
||||||
if ls.Service.TaggedAddresses == nil {
|
if svc.TaggedAddresses == nil {
|
||||||
ls.Service.TaggedAddresses = make(map[string]structs.ServiceAddress)
|
svc.TaggedAddresses = make(map[string]structs.ServiceAddress)
|
||||||
}
|
}
|
||||||
for k, v := range rs.TaggedAddresses {
|
for k, v := range rs.TaggedAddresses {
|
||||||
if strings.HasPrefix(k, structs.MetaKeyReservedPrefix) {
|
if strings.HasPrefix(k, structs.MetaKeyReservedPrefix) {
|
||||||
ls.Service.TaggedAddresses[k] = v
|
svc.TaggedAddresses[k] = v
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ls.InSync = ls.Service.IsSame(rs)
|
ls.InSync = svc.IsSame(rs)
|
||||||
|
|
||||||
|
// replace the service pointer to the new mutated struct
|
||||||
|
ls.Service = &svc
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check which checks need syncing
|
// Check which checks need syncing
|
||||||
|
|
|
@ -415,6 +415,12 @@ func TestAgentAntiEntropy_Services_ConnectProxy(t *testing.T) {
|
||||||
// All the services should match
|
// All the services should match
|
||||||
for id, serv := range services.NodeServices.Services {
|
for id, serv := range services.NodeServices.Services {
|
||||||
serv.CreateIndex, serv.ModifyIndex = 0, 0
|
serv.CreateIndex, serv.ModifyIndex = 0, 0
|
||||||
|
if serv.TaggedAddresses != nil {
|
||||||
|
serviceVIP := serv.TaggedAddresses[structs.TaggedAddressVirtualIP].Address
|
||||||
|
assert.NotEmpty(serviceVIP)
|
||||||
|
vips[serviceVIP] = struct{}{}
|
||||||
|
}
|
||||||
|
serv.TaggedAddresses = nil
|
||||||
switch id {
|
switch id {
|
||||||
case "mysql-proxy":
|
case "mysql-proxy":
|
||||||
assert.Equal(srv1, serv)
|
assert.Equal(srv1, serv)
|
||||||
|
|
Loading…
Reference in New Issue