diff --git a/tailnet/configmaps.go b/tailnet/configmaps.go index 92d123cea2..8de2adefa1 100644 --- a/tailnet/configmaps.go +++ b/tailnet/configmaps.go @@ -494,9 +494,15 @@ func (c *configMaps) updatePeerLocked(update *proto.CoordinateResponse_PeerUpdat lc.readyForHandshakeTimer.Stop() } if lc.node != nil { - old := lc.node.KeepAlive - lc.node.KeepAlive = c.nodeKeepalive(lc, status, lc.node) - dirty = dirty || (old != lc.node.KeepAlive) + // Replace rather than mutate: a stored node is shared outside c.L + // (log entries in a flight recorder, peer diagnostics), so it must not + // change after it is stored. + if keepAlive := c.nodeKeepalive(lc, status, lc.node); keepAlive != lc.node.KeepAlive { + next := lc.node.Clone() + next.KeepAlive = keepAlive + lc.node = next + dirty = true + } } logger.Debug(context.Background(), "peer ready for handshake") // only force a reconfig if the node populated diff --git a/tailnet/configmaps_internal_test.go b/tailnet/configmaps_internal_test.go index 169dd05d8c..b06029704d 100644 --- a/tailnet/configmaps_internal_test.go +++ b/tailnet/configmaps_internal_test.go @@ -335,6 +335,11 @@ func TestConfigMaps_updatePeers_new_waitForHandshake(t *testing.T) { // it should not send the peer to the netmap yet + uut.L.Lock() + preHandshake := uut.peers[p1ID].node + uut.L.Unlock() + require.False(t, preHandshake.KeepAlive) + go func() { <-fEng.status fEng.statusDone <- struct{}{} @@ -359,6 +364,15 @@ func TestConfigMaps_updatePeers_new_waitForHandshake(t *testing.T) { require.Equal(t, p1Node.Endpoints, n1.Endpoints) require.True(t, n1.KeepAlive) + // The stored node is replaced, not mutated, because it may be shared + // outside c.L (log entries, peer diagnostics). + require.False(t, preHandshake.KeepAlive, "stored node must not be mutated in place") + uut.L.Lock() + postHandshake := uut.peers[p1ID].node + uut.L.Unlock() + require.NotSame(t, preHandshake, postHandshake) + require.True(t, postHandshake.KeepAlive) + // we rely on nmcfg.WGCfg() to convert the netmap to wireguard config, so just // require the right number of peers. require.Len(t, r.wg.Peers, 1)