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 <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go
index 3a903f8f73479188504b303bdd23bb771948ec98..09adb9bdca4cd7319ab0dedf058a050775f19adc 100644 (file)
--- 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()
                                }()