From: Bryan C. Mills <bcmills@google.com>
Date: Sat, 20 Jan 2024 00:05:53 +0000 (-0500)
Subject: net: attempt to deflake TestListenCloseListen
X-Git-Tag: go1.23rc1~1431
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=41c05ea4b937ceb7b4eb7c3599f144f95da5e04e;p=gostls13.git

net: attempt to deflake TestListenCloseListen

Also make it flakier in longtest mode by burning through more
ephemeral ports. (Burning through the ports raised the failure rate
for me locally enough to reliably reproduce the failure in #65175 with
-count=10.)

Fixes #65175 (I hope).

Change-Id: I5f5b68b6bf6a6aa92e66f0288078817041656a3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/557177
Reviewed-by: Damien Neil <dneil@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/net_test.go b/src/net/net_test.go
index b448a79cce..f2fa848442 100644
--- a/src/net/net_test.go
+++ b/src/net/net_test.go
@@ -11,6 +11,7 @@ import (
 	"net/internal/socktest"
 	"os"
 	"runtime"
+	"sync"
 	"testing"
 	"time"
 )
@@ -294,27 +295,72 @@ func TestPacketConnClose(t *testing.T) {
 }
 
 func TestListenCloseListen(t *testing.T) {
-	const maxTries = 10
-	for tries := 0; tries < maxTries; tries++ {
-		ln := newLocalListener(t, "tcp")
-		addr := ln.Addr().String()
-		// TODO: This is racy. The selected address could be reused in between this
-		// Close and the subsequent Listen.
-		if err := ln.Close(); err != nil {
-			if perr := parseCloseError(err, false); perr != nil {
-				t.Error(perr)
-			}
-			t.Fatal(err)
+	if testing.Short() {
+		t.Parallel()
+	}
+
+	ln := newLocalListener(t, "tcp")
+	addr := ln.Addr().String()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		for {
+			c, err := ln.Accept()
+			if err != nil {
+				wg.Done()
+				return
+			}
+			wg.Add(1)
+			go func() {
+				io.Copy(io.Discard, c)
+				c.Close()
+				wg.Done()
+			}()
 		}
-		ln, err := Listen("tcp", addr)
-		if err == nil {
-			// Success. (This test didn't always make it here earlier.)
-			ln.Close()
-			return
+	}()
+	defer wg.Wait()
+
+	// Keep a connection alive while we close the listener to try to discourage
+	// the kernel from reusing the listener's port for some other process.
+	//
+	// TODO(bcmills): This empirically seems to work, and we also rely on it in
+	// TestDialClosedPortFailFast, but I can't find a reference documenting this
+	// port-reuse behavior.
+	c, err := Dial("tcp", addr)
+	defer c.Close()
+
+	if err := ln.Close(); err != nil {
+		if perr := parseCloseError(err, false); perr != nil {
+			t.Error(perr)
+		}
+		t.Fatal(err)
+	}
+
+	if !testing.Short() {
+		// Burn through some ephemeral ports (without actually accepting any
+		// connections on them) to try to encourage the kernel to reuse the address
+		// if it is going to.
+		lns := make(chan []Listener, 1)
+		lns <- nil
+		for i := 0; i < 4000; i++ {
+			ln := newLocalListener(t, "tcp")
+			lns <- append(<-lns, ln)
 		}
-		t.Errorf("failed on try %d/%d: %v", tries+1, maxTries, err)
+		defer func() {
+			for _, ln := range <-lns {
+				ln.Close()
+			}
+		}()
+	}
+
+	ln, err = Listen("tcp", addr)
+	if err == nil {
+		// Success. (This test didn't always make it here earlier.)
+		ln.Close()
+		return
 	}
-	t.Fatalf("failed to listen/close/listen on same address after %d tries", maxTries)
+	t.Fatalf("failed to listen/close/listen on same address")
 }
 
 // See golang.org/issue/6163, golang.org/issue/6987.