From d9ad3da588ffb39af619ace7ee73a44a9d77a56e Mon Sep 17 00:00:00 2001 From: AnonyScorpio Date: Sun, 16 Aug 2026 16:23:20 +0800 Subject: [PATCH] net: shuffle resolved IP addresses for load balancing Resolved addresses are now shuffled before sorting per RFC 6724, so concurrent callers do not always receive the same order. This helps distribute network connections across multiple addresses. The address slice is now always cloned before returning, allowing the caller to mutate it safely even when results are shared by concurrent singleflight callers. Fixes #31698 --- src/net/dnsclient.go | 11 +++++ src/net/dnsclient_unix.go | 8 +++- src/net/hook.go | 2 + src/net/lookup.go | 16 ++++++- src/net/lookup_test.go | 95 ++++++++++++++++++++++++++++++++++++++- src/net/lookup_unix.go | 5 ++- 6 files changed, 130 insertions(+), 7 deletions(-) diff --git a/src/net/dnsclient.go b/src/net/dnsclient.go index eb509d175fc1a0..497d4a38d88ed9 100644 --- a/src/net/dnsclient.go +++ b/src/net/dnsclient.go @@ -27,6 +27,17 @@ func randIntn(n int) int { return randInt() % n } +// shuffle is a copy of math/rand/v2.Shuffle. +func shuffle(n int, swap func(i, j int)) { + if n < 0 { + panic("invalid argument to Shuffle") + } + for i := n - 1; i > 0; i-- { + j := randIntn(i + 1) + swap(i, j) + } +} + // reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP // address addr suitable for rDNS (PTR) record lookup or an error if it fails // to parse the IP address. diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go index 6e749882c25c6a..d48f9343500146 100644 --- a/src/net/dnsclient_unix.go +++ b/src/net/dnsclient_unix.go @@ -624,7 +624,6 @@ func goLookupIPFiles(name string) (addrs []IPAddr, canonical string) { addrs = append(addrs, addr) } } - sortByRFC6724(addrs) return addrs, canonical } @@ -636,6 +635,12 @@ func (r *Resolver) goLookupIP(ctx context.Context, network, host string, order h } func (r *Resolver) goLookupIPCNAMEOrder(ctx context.Context, network, name string, order hostLookupOrder, conf *dnsConfig) (addrs []IPAddr, cname dnsmessage.Name, err error) { + addrs, cname, err = r.goLookupIPCNAME(ctx, network, name, order, conf) + sortByRFC6724(addrs) + return addrs, cname, err +} + +func (r *Resolver) goLookupIPCNAME(ctx context.Context, network, name string, order hostLookupOrder, conf *dnsConfig) (addrs []IPAddr, cname dnsmessage.Name, err error) { if order == hostLookupFilesDNS || order == hostLookupFiles { var canonical string addrs, canonical = goLookupIPFiles(name) @@ -825,7 +830,6 @@ func (r *Resolver) goLookupIPCNAMEOrder(ctx context.Context, network, name strin // just one is misleading. See also golang.org/issue/6324. lastErr.Name = name } - sortByRFC6724(addrs) if len(addrs) == 0 && !(network == "CNAME" && cname.Length > 0) { if order == hostLookupDNSFiles { var canonical string diff --git a/src/net/hook.go b/src/net/hook.go index 08d1aa893481f2..9a8252bdccea40 100644 --- a/src/net/hook.go +++ b/src/net/hook.go @@ -28,4 +28,6 @@ var ( // short deadline (such as 1ns in the future) is always expired by the time // a relevant system call occurs. testHookStepTime = func() {} + + testHookShuffleRand = shuffle ) diff --git a/src/net/lookup.go b/src/net/lookup.go index 06e14dfdda796f..a3c250bf27ab97 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -373,18 +373,30 @@ func (r *Resolver) lookupIPAddr(ctx context.Context, network, host string) ([]IP addrs, _ := r.Val.([]IPAddr) trace.DNSDone(ipAddrsEface(addrs), r.Shared, err) } - return lookupIPReturn(r.Val, err, r.Shared) + // Shuffled before sorting per RFC 6724, so concurrent callers do not always receive the same order. + // See https://go.dev/issue/34511. + // See https://go.dev/issue/31698. + addrs, err := lookupIPReturn(r.Val, err, r.Shared) + testHookShuffleRand(len(addrs), func(i, j int) { + addrs[i], addrs[j] = addrs[j], addrs[i] + }) + sortByRFC6724(addrs) + return addrs, err } } // lookupIPReturn turns the return values from singleflight.Do into // the return values from LookupIP. +// +// The caller may shuffle and sort the result in place, +// and the underlying slice is shared among all concurrent singleflight callers, +// so it must not be mutated directly. func lookupIPReturn(addrsi any, err error, shared bool) ([]IPAddr, error) { if err != nil { return nil, err } addrs := addrsi.([]IPAddr) - if shared { + if len(addrs) > 1 && shared { clone := make([]IPAddr, len(addrs)) copy(clone, addrs) addrs = clone diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index afa7e4c14aaf1f..8964db447005c6 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "internal/testenv" + "math/rand/v2" "net/netip" "reflect" "runtime" @@ -17,6 +18,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" ) @@ -1132,12 +1134,43 @@ func TestLookupIPAddrPreservesContextValues(t *testing.T) { if err != nil { t.Errorf("Resolver #%d: unexpected error: %v", i, err) } - if !reflect.DeepEqual(gotIPs, wantIPs) { + // Ignore order + if !reflect.DeepEqual(sortedIPAddrStrings(gotIPs), sortedIPAddrStrings(wantIPs)) { t.Errorf("#%d: mismatched IPAddr results\n\tGot: %v\n\tWant: %v", i, gotIPs, wantIPs) } } } +type lockedRand struct { + mu sync.Mutex + r *rand.Rand +} + +func newLockedRand(seed1, seed2 uint64) *lockedRand { + return &lockedRand{r: rand.New(rand.NewPCG(seed1, seed2))} +} + +func (lr *lockedRand) IntN(n int) int { + lr.mu.Lock() + defer lr.mu.Unlock() + return lr.r.IntN(n) +} + +func (lr *lockedRand) Shuffle(n int, swap func(i, j int)) { + lr.mu.Lock() + defer lr.mu.Unlock() + lr.r.Shuffle(n, swap) +} + +func sortedIPAddrStrings(ipAddrs []IPAddr) []string { + ret := make([]string, len(ipAddrs)) + for i, ipAddr := range ipAddrs { + ret[i] = ipAddr.String() + "\000" + ipAddr.Zone + } + slices.Sort(ret) + return ret +} + // Issue 30521: The lookup group should call the resolver for each network. func TestLookupIPAddrConcurrentCallsForNetworks(t *testing.T) { origTestHookLookupIP := testHookLookupIP @@ -1193,7 +1226,8 @@ func TestLookupIPAddrConcurrentCallsForNetworks(t *testing.T) { t.Errorf("lookupIPAddr(%v, %v): unexpected error: %v", network, host, err) } wantIPs := results[[2]string{network, host}] - if !reflect.DeepEqual(gotIPs, wantIPs) { + // Ignore order + if !reflect.DeepEqual(sortedIPAddrStrings(gotIPs), sortedIPAddrStrings(wantIPs)) { t.Errorf("lookupIPAddr(%v, %v): mismatched IPAddr results\n\tGot: %v\n\tWant: %v", network, host, gotIPs, wantIPs) } }() @@ -1201,6 +1235,63 @@ func TestLookupIPAddrConcurrentCallsForNetworks(t *testing.T) { wg.Wait() } +// Issue 31698: Concurrent callers do not always receive the same order. +func TestLookupIPAddrConcurrentCallsForShuffle(t *testing.T) { + synctest.Test(t, func(*testing.T) { + origTestHookLookupIP := testHookLookupIP + defer func() { testHookLookupIP = origTestHookLookupIP }() + + origTestHookShuffleRand := testHookShuffleRand + defer func() { testHookShuffleRand = origTestHookShuffleRand }() + + // Both addresses share the same commonPrefixLen, so the RFC 6724 sort calls + // Shuffle only once; with a fixed seed the pseudo-random result is predictable. + ipv4LocalHost := []IPAddr{ + {IP: IPv4(127, 0, 0, 2)}, + {IP: IPv4(127, 0, 0, 3)}, + } + testHookLookupIP = func(ctx context.Context, fn func(context.Context, string, string) ([]IPAddr, error), network, host string) ([]IPAddr, error) { + // Simulation for DNS query time cost + time.Sleep(50 * time.Millisecond) + return ipv4LocalHost, nil + } + // Use a locked random source so concurrent Shuffle calls do not race on shared state. + // With this deliberately chosen fixed seed the two concurrent lookups + // stay deterministic, so just two lookups suffice to verify each caller + // receives a shuffled result. + testHookShuffleRand = newLockedRand(0, 0).Shuffle + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + network := "udp" + host := "golang.org" + result := make([][]IPAddr, 0, 2) + var mu sync.Mutex + for range 2 { + go func() { + gotIPs, err := DefaultResolver.lookupIPAddr(ctx, network, host) + if err != nil { + t.Errorf("lookupIPAddr(%v, %v): unexpected error: %v", network, host, err) + } + // Ignore order + if !reflect.DeepEqual(sortedIPAddrStrings(gotIPs), sortedIPAddrStrings(ipv4LocalHost)) { + t.Errorf("lookupIPAddr(%v, %v): mismatched IPAddr results\n\tGot: %v\n\tWant: %v", network, host, gotIPs, ipv4LocalHost) + } + mu.Lock() + defer mu.Unlock() + result = append(result, gotIPs) + }() + } + synctest.Sleep(50 * time.Millisecond) + for i := 1; i < len(result); i++ { + // Lookups always return the same order (expected shuffled results) + if !reflect.DeepEqual(result[i], result[0]) { + return + } + } + t.Errorf("Lookups always return the same order (expected shuffled results)") + }) +} + // Issue 53995: Resolver.LookupIP should return error for empty host name. func TestResolverLookupIPWithEmptyHost(t *testing.T) { _, err := DefaultResolver.LookupIP(context.Background(), "ip", "") diff --git a/src/net/lookup_unix.go b/src/net/lookup_unix.go index 86108939cd03d6..d8fe2ef5c8e491 100644 --- a/src/net/lookup_unix.go +++ b/src/net/lookup_unix.go @@ -63,7 +63,10 @@ func (r *Resolver) lookupIP(ctx context.Context, network, host string) (addrs [] if order == hostLookupCgo { return cgoLookupIP(ctx, network, host) } - ips, _, err := r.goLookupIPCNAMEOrder(ctx, network, host, order, conf) + // Keep the resolver's addresses in their original order: the result + // may be shared by concurrent callers via singleflight, and is + // shuffled and sorted by RFC 6724 in lookupIPAddr. + ips, _, err := r.goLookupIPCNAME(ctx, network, host, order, conf) return ips, err }