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 652f59ede7f02bee6cebc7bcdf1afa07a1413441..88f8d34e1a5c10c472eae1b6d4cbb69d8bdd1b46 100644 (file)
--- 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 c2a85db6deb849ab92d9fe35367301f09989c31f..2a15845ea10f251fa426ebd22b3c822954d4459e 100644 (file)
--- 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()