net: delete TestListenCloseListen

In CL 557177, I attempted to fix a logical race in this test (#65175).
However, I introduced a data race in the process (#65209).

The race was reported on the windows-amd64-race builder. When I tried
to reproduce it on linux/amd64, I added a time.Sleep in the Accept
loop. However, that Sleep causes the test to fail outright with
EADDRINUSE, which suggests that my earlier guess about the open Conn
preventing reuse of the port was, in fact, incorrect.

On some platforms we could instead use SO_REUSEPORT and avoid closing
the first Listener entirely, but that wouldn't be even remotely in the
spirit of the original test.

Since I don't see a way to preserve the test in a way that is not
inherently flaky / racy, I suggest that we just delete it. It was
originally added as a regression test for a bug in the nacl port,
which no longer exists anyway. (Some of that code may live on in the
wasm port, but it doesn't seem worth maintaining a flaky
port-independent test to maintain a regression test for a bug specific
to secondary platforms.)

Fixes #65209.
Updates #65175.

Change-Id: I32f9da779d24f2e133571f0971ec460cebe7820a
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-race
Reviewed-on: https://go-review.googlesource.com/c/go/+/557536
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>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>

diff --git a/src/net/net_test.go b/src/net/net_test.go
index f2fa84844219ed117cbb3d429fd5a64059fd574e..4a5dc3b73a6b440b0d69ce1562e1841b19d0e9fd 100644 (file)
--- a/src/net/net_test.go
+++ b/src/net/net_test.go
@@ -11,7 +11,6 @@ import (
        "net/internal/socktest"
        "os"
        "runtime"
-       "sync"
        "testing"
        "time"
 )
@@ -294,75 +293,6 @@ func TestPacketConnClose(t *testing.T) {
        }
 }
 
-func TestListenCloseListen(t *testing.T) {
-       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()
-                       }()
-               }
-       }()
-       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)
-               }
-               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")
-}
-
 // See golang.org/issue/6163, golang.org/issue/6987.
 func TestAcceptIgnoreAbortedConnRequest(t *testing.T) {
        switch runtime.GOOS {