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 <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: Bryan C. Mills

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
+}

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)
+}

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)
+}

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++
 			}
-		}
+		})
 	}
 }