From 975a74774c737b4a3d2f36f4c69e1b4857d3f3f5 Mon Sep 17 00:00:00 2001 From: David Bimmler Date: Mon, 9 Sep 2024 13:37:17 +0200 Subject: [PATCH] client: make conn.UDPSize atomic to pacify -race As is, Conn is not safe to access concurrently, and the race detector correctly identifies occurrences of this, by reporting races in cilium/cilium: Read at 0x00c00754eb90 by goroutine 3305340: github.com/cilium/dns.(*Conn).ReadMsgHeader() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/client.go:304 +0xc4 github.com/cilium/dns.(*Conn).ReadMsg() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/client.go:274 +0x3d github.com/cilium/dns.handler.func2() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/shared_client.go:173 +0x15a Previous write at 0x00c00754eb90 by goroutine 3305339: github.com/cilium/dns.(*Client).SendContext() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/client.go:242 +0xfe github.com/cilium/dns.handler() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/shared_client.go:251 +0x773 github.com/cilium/dns.(*SharedClient).ExchangeSharedContext.gowrap1() /go/src/github.com/cilium/cilium/vendor/github.com/cilium/dns/shared_client.go:298 +0x5d By making the size an atomic uint, the race detector will be pacified. Signed-off-by: David Bimmler --- client.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client.go b/client.go index f689b37a0c..4b12eb46fc 100644 --- a/client.go +++ b/client.go @@ -10,6 +10,7 @@ import ( "io" "net" "strings" + "sync/atomic" "time" ) @@ -33,7 +34,7 @@ func isPacketConn(c net.Conn) bool { // A Conn represents a connection to a DNS server. type Conn struct { net.Conn // a net.Conn holding the connection - UDPSize uint16 // minimum receive buffer for UDP messages + UDPSize atomic.Uint32 // minimum receive buffer for UDP messages (actually uint16, but there's no atomic variant) TsigSecret map[string]string // secret(s) for Tsig map[], zonename must be in canonical form (lowercase, fqdn, see RFC 4034 Section 6.2) TsigProvider TsigProvider // An implementation of the TsigProvider interface. If defined it replaces TsigSecret and is used for all TSIG operations. tsigRequestMAC string @@ -142,7 +143,7 @@ func (c *Client) DialContext(ctx context.Context, address string) (conn *Conn, e if err != nil { return nil, err } - conn.UDPSize = c.UDPSize + conn.UDPSize.Store(uint32(c.UDPSize)) return conn, nil } @@ -239,11 +240,11 @@ func (c *Client) SendContext(ctx context.Context, m *Msg, co *Conn, t time.Time) opt := m.IsEdns0() // If EDNS0 is used use that for size. if opt != nil && opt.UDPSize() >= MinMsgSize { - co.UDPSize = opt.UDPSize() + co.UDPSize.Store(uint32(opt.UDPSize())) } // Otherwise use the client's configured UDP size. if opt == nil && c.UDPSize >= MinMsgSize { - co.UDPSize = c.UDPSize + co.UDPSize.Store(uint32(c.UDPSize)) } // write with the appropriate write timeout @@ -301,8 +302,8 @@ func (co *Conn) ReadMsgHeader(hdr *Header) ([]byte, error) { ) if isPacketConn(co.Conn) { - if co.UDPSize > MinMsgSize { - p = make([]byte, co.UDPSize) + if us := co.UDPSize.Load(); us > MinMsgSize { + p = make([]byte, us) } else { p = make([]byte, MinMsgSize) } @@ -436,7 +437,6 @@ func ExchangeContext(ctx context.Context, m *Msg, a string) (r *Msg, err error) // co.WriteMsg(m) // in, _ := co.ReadMsg() // co.Close() -// func ExchangeConn(c net.Conn, m *Msg) (r *Msg, err error) { println("dns: ExchangeConn: this function is deprecated") co := new(Conn)