net: refactor TestWriteToTimeout

The test cases for this test had listed specific errors, but the
specific error values were ignored in favor of just calling
isDeadlineExceeded.

Moreover, ENOBUFS errors (which can legitimately occur in the test if
the network interface also happens to be saturated when the timeout
occurs) were not handled at all.

Now the test relies only on the timeout: we iterate until we have seen
two of the expected timeout errors, and if we see ENOBUFS instead of
"deadline exceeded" we back off to give the queues time to drain.

Fixes #49930

Change-Id: I258a6d5c935d9635b02dffd79e197ba9caf83ac8
Reviewed-on: https://go-review.googlesource.com/c/go/+/370882
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/net/error_plan9_test.go b/src/net/error_plan9_test.go
index d7c7f1487fb14321e5cf1275aad7ad003155e71b..1270af19e54d5bcf53e44b99c891cb96b0183476 100644 (file)
--- a/src/net/error_plan9_test.go
+++ b/src/net/error_plan9_test.go
@@ -17,3 +17,7 @@ func isPlatformError(err error) bool {
        _, ok := err.(syscall.ErrorString)
        return ok
 }
+
+func isENOBUFS(err error) bool {
+       return false // ENOBUFS is Unix-specific
+}

diff --git a/src/net/error_unix_test.go b/src/net/error_unix_test.go
index 1334aa86b6882f5325a07caf5b7b98d1bf7732b7..291a7234f2155574318e523e0c8c74feb1824d13 100644 (file)
--- a/src/net/error_unix_test.go
+++ b/src/net/error_unix_test.go
@@ -7,6 +7,7 @@
 package net
 
 import (
+       "errors"
        "os"
        "syscall"
 )
@@ -32,3 +33,7 @@ func samePlatformError(err, want error) bool {
        }
        return err == want
 }
+
+func isENOBUFS(err error) bool {
+       return errors.Is(err, syscall.ENOBUFS)
+}

diff --git a/src/net/error_windows_test.go b/src/net/error_windows_test.go
index 834a9de4411a015412d49c74c1b93e11d0daac72..25825f96f807534241a6c56433383fd9f5256b24 100644 (file)
--- a/src/net/error_windows_test.go
+++ b/src/net/error_windows_test.go
@@ -4,7 +4,10 @@
 
 package net
 
-import "syscall"
+import (
+       "errors"
+       "syscall"
+)
 
 var (
        errTimedout       = syscall.ETIMEDOUT
@@ -17,3 +20,10 @@ func isPlatformError(err error) bool {
        _, ok := err.(syscall.Errno)
        return ok
 }
+
+func isENOBUFS(err error) bool {
+       // syscall.ENOBUFS is a completely made-up value on Windows: we don't expect
+       // a real system call to ever actually return it. However, since it is already
+       // defined in the syscall package we may as well check for it.
+       return errors.Is(err, syscall.ENOBUFS)
+}

diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go
index cd6b95374795625d38c6e34fb40f1a479847577a..032770dd8348f516fb71bc13dee1a5ec9c242ca7 100644 (file)
--- a/src/net/timeout_test.go
+++ b/src/net/timeout_test.go
@@ -557,17 +557,6 @@ func TestWriteTimeoutMustNotReturn(t *testing.T) {
        }
 }
 
-var writeToTimeoutTests = []struct {
-       timeout time.Duration
-       xerrs   [2]error // expected errors in transition
-}{
-       // Tests that write deadlines work, even if there's buffer
-       // space available to write.
-       {-5 * time.Second, [2]error{os.ErrDeadlineExceeded, os.ErrDeadlineExceeded}},
-
-       {10 * time.Millisecond, [2]error{nil, os.ErrDeadlineExceeded}},
-}
-
 func TestWriteToTimeout(t *testing.T) {
        t.Parallel()
 
@@ -579,37 +568,58 @@ func TestWriteToTimeout(t *testing.T) {
                t.Fatal(err)
        }
 
-       for i, tt := range writeToTimeoutTests {
-               c2, err := ListenPacket(c1.LocalAddr().Network(), JoinHostPort(host, "0"))
-               if err != nil {
-                       t.Fatal(err)
-               }
-               defer c2.Close()
+       timeouts := []time.Duration{
+               -5 * time.Second,
+               10 * time.Millisecond,
+       }
 
-               if err := c2.SetWriteDeadline(time.Now().Add(tt.timeout)); err != nil {
-                       t.Fatalf("#%d: %v", i, err)
-               }
-               for j, xerr := range tt.xerrs {
-                       for {
+       for _, timeout := range timeouts {
+               t.Run(fmt.Sprint(timeout), func(t *testing.T) {
+                       c2, err := ListenPacket(c1.LocalAddr().Network(), JoinHostPort(host, "0"))
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+                       defer c2.Close()
+
+                       if err := c2.SetWriteDeadline(time.Now().Add(timeout)); err != nil {
+                               t.Fatalf("SetWriteDeadline: %v", err)
+                       }
+                       backoff := 1 * time.Millisecond
+                       nDeadlineExceeded := 0
+                       for j := 0; nDeadlineExceeded < 2; j++ {
                                n, err := c2.WriteTo([]byte("WRITETO TIMEOUT TEST"), c1.LocalAddr())
-                               if xerr != nil {
-                                       if perr := parseWriteError(err); perr != nil {
-                                               t.Errorf("#%d/%d: %v", i, j, perr)
-                                       }
-                                       if !isDeadlineExceeded(err) {
-                                               t.Fatalf("#%d/%d: %v", i, j, err)
-                                       }
+                               t.Logf("#%d: WriteTo: %d, %v", j, n, err)
+                               if err == nil && timeout >= 0 && nDeadlineExceeded == 0 {
+                                       // If the timeout is nonnegative, some number of WriteTo calls may
+                                       // succeed before the timeout takes effect.
+                                       t.Logf("WriteTo succeeded; sleeping %v", timeout/3)
+                                       time.Sleep(timeout / 3)
+                                       continue
                                }
-                               if err == nil {
-                                       time.Sleep(tt.timeout / 3)
+                               if isENOBUFS(err) {
+                                       t.Logf("WriteTo: %v", err)
+                                       // We're looking for a deadline exceeded error, but if the kernel's
+                                       // network buffers are saturated we may see ENOBUFS instead (see
+                                       // https://go.dev/issue/49930). Give it some time to unsaturate.
+                                       time.Sleep(backoff)
+                                       backoff *= 2
                                        continue
                                }
+                               if perr := parseWriteError(err); perr != nil {
+                                       t.Errorf("failed to parse error: %v", perr)
+                               }
+                               if !isDeadlineExceeded(err) {
+                                       t.Errorf("error is not 'deadline exceeded'")
+                               }
                                if n != 0 {
-                                       t.Fatalf("#%d/%d: wrote %d; want 0", i, j, n)
+                                       t.Errorf("unexpectedly wrote %d bytes", n)
                                }
-                               break
+                               if !t.Failed() {
+                                       t.Logf("WriteTo timed out as expected")
+                               }
+                               nDeadlineExceeded++
                        }
-               }
+               })
        }
 }