From: Ian Lance Taylor <iant@golang.org>
Date: Wed, 16 Nov 2022 22:19:47 +0000 (-0800)
Subject: net: change resolverConfig.dnsConfig to an atomic.Pointer
X-Git-Tag: go1.20rc1~200
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b7662047aedc5f2c512911eb59d514ce75b16e18;p=gostls13.git

net: change resolverConfig.dnsConfig to an atomic.Pointer

We were using a RWMutex RLock around a single memory load,
which is not a good use of a RWMutex--it introduces extra work
for the RLock but contention around a single memory load is unlikely.
And, the tryUpdate method was not acquiring the mutex anyhow.

The new atomic.Pointer type is type-safe and easy to use correctly
for a simple use-case like this.

Change-Id: Ib3859c03414c44d2e897f6d15c92c8e4b5c81a11
Reviewed-on: https://go-review.googlesource.com/c/go/+/451416
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
---

diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index 652f59ede7..88f8d34e1a 100644
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"runtime"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"golang.org/x/net/dns/dnsmessage"
@@ -342,25 +343,21 @@ type resolverConfig struct {
 	ch          chan struct{} // guards lastChecked and modTime
 	lastChecked time.Time     // last time resolv.conf was checked
 
-	mu        sync.RWMutex // protects dnsConfig
-	dnsConfig *dnsConfig   // parsed resolv.conf structure used in lookups
+	dnsConfig atomic.Pointer[dnsConfig] // parsed resolv.conf structure used in lookups
 }
 
 var resolvConf resolverConfig
 
 func getSystemDNSConfig() *dnsConfig {
 	resolvConf.tryUpdate("/etc/resolv.conf")
-	resolvConf.mu.RLock()
-	resolv := resolvConf.dnsConfig
-	resolvConf.mu.RUnlock()
-	return resolv
+	return resolvConf.dnsConfig.Load()
 }
 
 // init initializes conf and is only called via conf.initOnce.
 func (conf *resolverConfig) init() {
 	// Set dnsConfig and lastChecked so we don't parse
 	// resolv.conf twice the first time.
-	conf.dnsConfig = dnsReadConfig("/etc/resolv.conf")
+	conf.dnsConfig.Store(dnsReadConfig("/etc/resolv.conf"))
 	conf.lastChecked = time.Now()
 
 	// Prepare ch so that only one update of resolverConfig may
@@ -374,7 +371,7 @@ func (conf *resolverConfig) init() {
 func (conf *resolverConfig) tryUpdate(name string) {
 	conf.initOnce.Do(conf.init)
 
-	if conf.dnsConfig.noReload {
+	if conf.dnsConfig.Load().noReload {
 		return
 	}
 
@@ -402,15 +399,13 @@ func (conf *resolverConfig) tryUpdate(name string) {
 		if fi, err := os.Stat(name); err == nil {
 			mtime = fi.ModTime()
 		}
-		if mtime.Equal(conf.dnsConfig.mtime) {
+		if mtime.Equal(conf.dnsConfig.Load().mtime) {
 			return
 		}
 	}
 
 	dnsConf := dnsReadConfig(name)
-	conf.mu.Lock()
-	conf.dnsConfig = dnsConf
-	conf.mu.Unlock()
+	conf.dnsConfig.Store(dnsConf)
 }
 
 func (conf *resolverConfig) tryAcquireSema() bool {
diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index c2a85db6de..2a15845ea1 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -280,9 +280,7 @@ func (conf *resolvConfTest) forceUpdate(name string, lastChecked time.Time) erro
 }
 
 func (conf *resolvConfTest) forceUpdateConf(c *dnsConfig, lastChecked time.Time) bool {
-	conf.mu.Lock()
-	conf.dnsConfig = c
-	conf.mu.Unlock()
+	conf.dnsConfig.Store(c)
 	for i := 0; i < 5; i++ {
 		if conf.tryAcquireSema() {
 			conf.lastChecked = lastChecked
@@ -294,10 +292,7 @@ func (conf *resolvConfTest) forceUpdateConf(c *dnsConfig, lastChecked time.Time)
 }
 
 func (conf *resolvConfTest) servers() []string {
-	conf.mu.RLock()
-	servers := conf.dnsConfig.servers
-	conf.mu.RUnlock()
-	return servers
+	return conf.dnsConfig.Load().servers
 }
 
 func (conf *resolvConfTest) teardown() error {
@@ -1445,9 +1440,7 @@ func TestDNSGoroutineRace(t *testing.T) {
 func lookupWithFake(fake fakeDNSServer, name string, typ dnsmessage.Type) error {
 	r := Resolver{PreferGo: true, Dial: fake.DialContext}
 
-	resolvConf.mu.RLock()
-	conf := resolvConf.dnsConfig
-	resolvConf.mu.RUnlock()
+	conf := resolvConf.dnsConfig.Load()
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()