diff options
author | Ian Lance Taylor <iant@golang.org> | 2018-03-15 11:55:30 -0700 |
---|---|---|
committer | Andrew Bonventre <andybons@golang.org> | 2018-03-29 06:08:28 +0000 |
commit | df9d6204b92c6cba7b0b2104b570f20956dd6568 (patch) | |
tree | 2e3cd7f48b7d7cf46a5fe986e21d55ec21d9a9a6 | |
parent | 67b695627005fbd66183e00036450b5c639427a8 (diff) | |
download | go-df9d6204b92c6cba7b0b2104b570f20956dd6568.tar.gz go-df9d6204b92c6cba7b0b2104b570f20956dd6568.zip |
[release-branch.go1.10] net: don't let cancelation of a DNS lookup affect another lookup
Updates #8602
Updates #20703
Fixes #22724
Change-Id: I27b72311b2c66148c59977361bd3f5101e47b51d
Reviewed-on: https://go-review.googlesource.com/100840
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/102787
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
-rw-r--r-- | src/internal/singleflight/singleflight.go | 22 | ||||
-rw-r--r-- | src/net/lookup.go | 32 | ||||
-rw-r--r-- | src/net/lookup_test.go | 25 | ||||
-rw-r--r-- | src/net/tcpsock_unix_test.go | 1 |
4 files changed, 65 insertions, 15 deletions
diff --git a/src/internal/singleflight/singleflight.go b/src/internal/singleflight/singleflight.go index 1e9960d575..b2d82e26c2 100644 --- a/src/internal/singleflight/singleflight.go +++ b/src/internal/singleflight/singleflight.go @@ -103,11 +103,21 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) { g.mu.Unlock() } -// Forget tells the singleflight to forget about a key. Future calls -// to Do for this key will call the function rather than waiting for -// an earlier call to complete. -func (g *Group) Forget(key string) { +// ForgetUnshared tells the singleflight to forget about a key if it is not +// shared with any other goroutines. Future calls to Do for a forgotten key +// will call the function rather than waiting for an earlier call to complete. +// Returns whether the key was forgotten or unknown--that is, whether no +// other goroutines are waiting for the result. +func (g *Group) ForgetUnshared(key string) bool { g.mu.Lock() - delete(g.m, key) - g.mu.Unlock() + defer g.mu.Unlock() + c, ok := g.m[key] + if !ok { + return true + } + if c.dups == 0 { + delete(g.m, key) + return true + } + return false } diff --git a/src/net/lookup.go b/src/net/lookup.go index 85e472932f..a65b735f92 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -194,10 +194,16 @@ func (r *Resolver) LookupIPAddr(ctx context.Context, host string) ([]IPAddr, err resolverFunc = alt } + // We don't want a cancelation of ctx to affect the + // lookupGroup operation. Otherwise if our context gets + // canceled it might cause an error to be returned to a lookup + // using a completely different context. + lookupGroupCtx, lookupGroupCancel := context.WithCancel(context.Background()) + dnsWaitGroup.Add(1) ch, called := lookupGroup.DoChan(host, func() (interface{}, error) { defer dnsWaitGroup.Done() - return testHookLookupIP(ctx, resolverFunc, host) + return testHookLookupIP(lookupGroupCtx, resolverFunc, host) }) if !called { dnsWaitGroup.Done() @@ -205,20 +211,28 @@ func (r *Resolver) LookupIPAddr(ctx context.Context, host string) ([]IPAddr, err select { case <-ctx.Done(): - // If the DNS lookup timed out for some reason, force - // future requests to start the DNS lookup again - // rather than waiting for the current lookup to - // complete. See issue 8602. - ctxErr := ctx.Err() - if ctxErr == context.DeadlineExceeded { - lookupGroup.Forget(host) + // Our context was canceled. If we are the only + // goroutine looking up this key, then drop the key + // from the lookupGroup and cancel the lookup. + // If there are other goroutines looking up this key, + // let the lookup continue uncanceled, and let later + // lookups with the same key share the result. + // See issues 8602, 20703, 22724. + if lookupGroup.ForgetUnshared(host) { + lookupGroupCancel() + } else { + go func() { + <-ch + lookupGroupCancel() + }() } - err := mapErr(ctxErr) + err := mapErr(ctx.Err()) if trace != nil && trace.DNSDone != nil { trace.DNSDone(nil, false, err) } return nil, err case r := <-ch: + lookupGroupCancel() if trace != nil && trace.DNSDone != nil { addrs, _ := r.Val.([]IPAddr) trace.DNSDone(ipAddrsEface(addrs), r.Shared, r.Err) diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index bfb872551c..24787ccf2b 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -791,3 +791,28 @@ func TestLookupNonLDH(t *testing.T) { t.Fatalf("lookup error = %v, want %v", err, errNoSuchHost) } } + +func TestLookupContextCancel(t *testing.T) { + if testenv.Builder() == "" { + testenv.MustHaveExternalNetwork(t) + } + if runtime.GOOS == "nacl" { + t.Skip("skip on nacl") + } + + defer dnsWaitGroup.Wait() + + ctx, ctxCancel := context.WithCancel(context.Background()) + ctxCancel() + _, err := DefaultResolver.LookupIPAddr(ctx, "google.com") + if err != errCanceled { + testenv.SkipFlakyNet(t) + t.Fatal(err) + } + ctx = context.Background() + _, err = DefaultResolver.LookupIPAddr(ctx, "google.com") + if err != nil { + testenv.SkipFlakyNet(t) + t.Fatal(err) + } +} diff --git a/src/net/tcpsock_unix_test.go b/src/net/tcpsock_unix_test.go index 3af1834455..95c02d2721 100644 --- a/src/net/tcpsock_unix_test.go +++ b/src/net/tcpsock_unix_test.go @@ -87,6 +87,7 @@ func TestTCPSpuriousConnSetupCompletionWithCancel(t *testing.T) { if testenv.Builder() == "" { testenv.MustHaveExternalNetwork(t) } + defer dnsWaitGroup.Wait() t.Parallel() const tries = 10000 var wg sync.WaitGroup |