diff --git a/.changelog/11940.txt b/.changelog/11940.txt new file mode 100644 index 000000000..22278079f --- /dev/null +++ b/.changelog/11940.txt @@ -0,0 +1,3 @@ +```release-note:bug + Mutate `NodeService` struct properly to avoid a data race. +``` diff --git a/agent/local/state.go b/agent/local/state.go index 03db0badb..d8ad529f7 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -1082,27 +1082,34 @@ func (l *State) updateSyncState() error { 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 // copy so that we don't retain a pointer to any actual state // store info for in-memory RPCs. - if ls.Service.EnableTagOverride { - ls.Service.Tags = make([]string, len(rs.Tags)) - copy(ls.Service.Tags, rs.Tags) + if svc.EnableTagOverride { + svc.Tags = make([]string, len(rs.Tags)) + copy(svc.Tags, rs.Tags) } // Merge any tagged addresses with the consul- prefix (set by the server) // back into the local state. - if !reflect.DeepEqual(ls.Service.TaggedAddresses, rs.TaggedAddresses) { - if ls.Service.TaggedAddresses == nil { - ls.Service.TaggedAddresses = make(map[string]structs.ServiceAddress) + if !reflect.DeepEqual(svc.TaggedAddresses, rs.TaggedAddresses) { + if svc.TaggedAddresses == nil { + svc.TaggedAddresses = make(map[string]structs.ServiceAddress) } for k, v := range rs.TaggedAddresses { 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 diff --git a/agent/local/state_test.go b/agent/local/state_test.go index f2c51a281..679adba84 100644 --- a/agent/local/state_test.go +++ b/agent/local/state_test.go @@ -415,6 +415,12 @@ func TestAgentAntiEntropy_Services_ConnectProxy(t *testing.T) { // All the services should match for id, serv := range services.NodeServices.Services { 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 { case "mysql-proxy": assert.Equal(srv1, serv)