From 749ebaa4460af237f9facbc43af3e055802dc0be Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 23 Jan 2024 17:08:50 -0500 Subject: [PATCH] net: ignore Dial errors in TestAcceptTimeout Also use DialContext instead of just Dial so that we can ensure the call returns before we close the listener. The Dial in this test is intended to complete before the call to Accept, but there is no synchronization to ensure that and sometimes it doesn't happen. That's ok and mostly immaterial to the test, but it does mean we need to ignore Dial errors (which can happen when the listener is closed), and we need to be a little more careful about not dialing a port that may have already been reused by some other test. Fixes #65240. Updates #17948. Change-Id: Ife1b5c3062939441b58f4c096461bf5d7841889b Reviewed-on: https://go-review.googlesource.com/c/go/+/558175 Reviewed-by: Damien Neil TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills LUCI-TryBot-Result: Go LUCI --- src/net/timeout_test.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go index 3a903f8f73..09adb9bdca 100644 --- a/src/net/timeout_test.go +++ b/src/net/timeout_test.go @@ -5,6 +5,7 @@ package net import ( + "context" "errors" "fmt" "io" @@ -193,17 +194,34 @@ func TestAcceptTimeout(t *testing.T) { // incoming connections available. Try to make one available before the // call to Accept happens. (It's ok if the timing doesn't always work // out that way, though: the test should pass regardless.) + ctx, cancel := context.WithCancel(context.Background()) dialDone := make(chan struct{}) - t.Cleanup(func() { <-dialDone }) + + // Ensure that our background Dial returns before we close the listener. + // Otherwise, the listener's port could be reused immediately and we + // might spuriously Dial some completely unrelated socket, causing some + // other test to see an unexpected extra connection. + defer func() { + cancel() + <-dialDone + }() go func() { defer close(dialDone) d := Dialer{} - c, err := d.Dial(ln.Addr().Network(), ln.Addr().String()) + c, err := d.DialContext(ctx, ln.Addr().Network(), ln.Addr().String()) if err != nil { - t.Error(err) + // If the timing didn't work out, it is possible for this Dial + // to return an error (depending on the kernel's buffering behavior). + // In https://go.dev/issue/65240 we saw failures with ECONNREFUSED + // and ECONNRESET. + // + // What this test really cares about is the behavior of Accept, not + // Dial, so just log the error and ignore it. + t.Logf("DialContext: %v", err) return } + t.Logf("Dialed %v -> %v", c.LocalAddr(), c.RemoteAddr()) c.Close() }() -- 2.48.1