qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-26 00:28:00 -07:00
Code Releases Activity

net: delete TestListenCloseListen

Browse Source 
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>
This commit is contained in:
Bryan C. Mills 2024-01-22 13:54:56 -05:00 committed by Gopher Robot
parent e5eeadb2d8
commit 6e7aee5f6b
1 changed files with 0 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/net/net_test.go
View File

@ -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 {