Skip to content

Commit df9d620

Browse files
ianlancetaylorandybons
authored andcommitted
[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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-on: https://go-review.googlesource.com/102787 Run-TryBot: Andrew Bonventre <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 67b6956 commit df9d620

File tree

4 files changed

+65
-15
lines changed

4 files changed

+65
-15
lines changed

src/internal/singleflight/singleflight.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,21 @@ func (g *Group) doCall(c *call, key string, fn func() (interface{}, error)) {
103103
g.mu.Unlock()
104104
}
105105

106-
// Forget tells the singleflight to forget about a key. Future calls
107-
// to Do for this key will call the function rather than waiting for
108-
// an earlier call to complete.
109-
func (g *Group) Forget(key string) {
106+
// ForgetUnshared tells the singleflight to forget about a key if it is not
107+
// shared with any other goroutines. Future calls to Do for a forgotten key
108+
// will call the function rather than waiting for an earlier call to complete.
109+
// Returns whether the key was forgotten or unknown--that is, whether no
110+
// other goroutines are waiting for the result.
111+
func (g *Group) ForgetUnshared(key string) bool {
110112
g.mu.Lock()
111-
delete(g.m, key)
112-
g.mu.Unlock()
113+
defer g.mu.Unlock()
114+
c, ok := g.m[key]
115+
if !ok {
116+
return true
117+
}
118+
if c.dups == 0 {
119+
delete(g.m, key)
120+
return true
121+
}
122+
return false
113123
}

src/net/lookup.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -194,31 +194,45 @@ func (r *Resolver) LookupIPAddr(ctx context.Context, host string) ([]IPAddr, err
194194
resolverFunc = alt
195195
}
196196

197+
// We don't want a cancelation of ctx to affect the
198+
// lookupGroup operation. Otherwise if our context gets
199+
// canceled it might cause an error to be returned to a lookup
200+
// using a completely different context.
201+
lookupGroupCtx, lookupGroupCancel := context.WithCancel(context.Background())
202+
197203
dnsWaitGroup.Add(1)
198204
ch, called := lookupGroup.DoChan(host, func() (interface{}, error) {
199205
defer dnsWaitGroup.Done()
200-
return testHookLookupIP(ctx, resolverFunc, host)
206+
return testHookLookupIP(lookupGroupCtx, resolverFunc, host)
201207
})
202208
if !called {
203209
dnsWaitGroup.Done()
204210
}
205211

206212
select {
207213
case <-ctx.Done():
208-
// If the DNS lookup timed out for some reason, force
209-
// future requests to start the DNS lookup again
210-
// rather than waiting for the current lookup to
211-
// complete. See issue 8602.
212-
ctxErr := ctx.Err()
213-
if ctxErr == context.DeadlineExceeded {
214-
lookupGroup.Forget(host)
214+
// Our context was canceled. If we are the only
215+
// goroutine looking up this key, then drop the key
216+
// from the lookupGroup and cancel the lookup.
217+
// If there are other goroutines looking up this key,
218+
// let the lookup continue uncanceled, and let later
219+
// lookups with the same key share the result.
220+
// See issues 8602, 20703, 22724.
221+
if lookupGroup.ForgetUnshared(host) {
222+
lookupGroupCancel()
223+
} else {
224+
go func() {
225+
<-ch
226+
lookupGroupCancel()
227+
}()
215228
}
216-
err := mapErr(ctxErr)
229+
err := mapErr(ctx.Err())
217230
if trace != nil && trace.DNSDone != nil {
218231
trace.DNSDone(nil, false, err)
219232
}
220233
return nil, err
221234
case r := <-ch:
235+
lookupGroupCancel()
222236
if trace != nil && trace.DNSDone != nil {
223237
addrs, _ := r.Val.([]IPAddr)
224238
trace.DNSDone(ipAddrsEface(addrs), r.Shared, r.Err)

src/net/lookup_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,3 +791,28 @@ func TestLookupNonLDH(t *testing.T) {
791791
t.Fatalf("lookup error = %v, want %v", err, errNoSuchHost)
792792
}
793793
}
794+
795+
func TestLookupContextCancel(t *testing.T) {
796+
if testenv.Builder() == "" {
797+
testenv.MustHaveExternalNetwork(t)
798+
}
799+
if runtime.GOOS == "nacl" {
800+
t.Skip("skip on nacl")
801+
}
802+
803+
defer dnsWaitGroup.Wait()
804+
805+
ctx, ctxCancel := context.WithCancel(context.Background())
806+
ctxCancel()
807+
_, err := DefaultResolver.LookupIPAddr(ctx, "google.com")
808+
if err != errCanceled {
809+
testenv.SkipFlakyNet(t)
810+
t.Fatal(err)
811+
}
812+
ctx = context.Background()
813+
_, err = DefaultResolver.LookupIPAddr(ctx, "google.com")
814+
if err != nil {
815+
testenv.SkipFlakyNet(t)
816+
t.Fatal(err)
817+
}
818+
}

src/net/tcpsock_unix_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func TestTCPSpuriousConnSetupCompletionWithCancel(t *testing.T) {
8787
if testenv.Builder() == "" {
8888
testenv.MustHaveExternalNetwork(t)
8989
}
90+
defer dnsWaitGroup.Wait()
9091
t.Parallel()
9192
const tries = 10000
9293
var wg sync.WaitGroup

0 commit comments

Comments
 (0)