netutil: streamline and simplify LimitListener tests
TestLimitListener had made a lot of assumptions about the kernel's
willingness to queue unaccepted connections, and relied on arbitrary
timeouts to shed load if the queue saturates.
This change eliminates the arbitrary timeouts, replacing them with
synchronization and cancellation and leaving only a couple of
arbitrary sleeps (that can be exceeded by arbitrary amounts without
causing the test to fail).
Fixes golang/go#22926
Change-Id: Ibecff6254ec966e1cc98cf96c71493f18d3aaebe
Reviewed-on: https://go-review.googlesource.com/c/net/+/372495
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/netutil/helper_stub_test.go b/netutil/helper_stub_test.go
deleted file mode 100644
index 863d23b..0000000
--- a/netutil/helper_stub_test.go
+++ /dev/null
@@ -1,12 +0,0 @@
-// Copyright 2014 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !windows
-// +build !aix,!darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris,!windows
-
-package netutil
-
-func maxOpenFiles() int {
- return defaultMaxOpenFiles
-}
diff --git a/netutil/helper_unix_test.go b/netutil/helper_unix_test.go
deleted file mode 100644
index 922790a..0000000
--- a/netutil/helper_unix_test.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2015 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
-
-package netutil
-
-import "syscall"
-
-func maxOpenFiles() int {
- var rlim syscall.Rlimit
- if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rlim); err != nil {
- return defaultMaxOpenFiles
- }
- return int(rlim.Cur)
-}
diff --git a/netutil/helper_windows_test.go b/netutil/helper_windows_test.go
deleted file mode 100644
index 8d5703d..0000000
--- a/netutil/helper_windows_test.go
+++ /dev/null
@@ -1,9 +0,0 @@
-// Copyright 2015 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package netutil
-
-func maxOpenFiles() int {
- return 4 * defaultMaxOpenFiles /* actually it's 16581375 */
-}
diff --git a/netutil/listen_test.go b/netutil/listen_test.go
index 0108e8d..ab8e599 100644
--- a/netutil/listen_test.go
+++ b/netutil/listen_test.go
@@ -5,73 +5,104 @@
package netutil
import (
+ "context"
"errors"
- "fmt"
"io"
- "io/ioutil"
"net"
- "net/http"
- "runtime"
"sync"
"sync/atomic"
"testing"
"time"
)
-const defaultMaxOpenFiles = 256
-const timeout = 5 * time.Second
-
func TestLimitListener(t *testing.T) {
- if runtime.GOOS == "plan9" {
- t.Skipf("skipping test known to be flaky on plan9 (https://golang.org/issue/22926)")
- }
-
- const max = 5
- attempts := (maxOpenFiles() - max) / 2
- if attempts > 256 { // maximum length of accept queue is 128 by default
- attempts = 256
- }
+ const (
+ max = 5
+ attempts = max * 2
+ msg = "bye\n"
+ )
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
- defer l.Close()
l = LimitListener(l, max)
- var open int32
- go http.Serve(l, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- if n := atomic.AddInt32(&open, 1); n > max {
- t.Errorf("%d open connections, want <= %d", n, max)
- }
- defer atomic.AddInt32(&open, -1)
- time.Sleep(10 * time.Millisecond)
- fmt.Fprint(w, "some body")
- }))
-
var wg sync.WaitGroup
- var failed int32
- for i := 0; i < attempts; i++ {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+
+ accepted := 0
+ for {
+ c, err := l.Accept()
+ if err != nil {
+ break
+ }
+ accepted++
+ io.WriteString(c, msg)
+
+ defer c.Close() // Leave c open until the listener is closed.
+ }
+ if accepted > max {
+ t.Errorf("accepted %d simultaneous connections; want at most %d", accepted, max)
+ }
+ }()
+
+ // connc keeps the client end of the dialed connections alive until the
+ // test completes.
+ connc := make(chan []net.Conn, 1)
+ connc <- nil
+
+ dialCtx, cancelDial := context.WithCancel(context.Background())
+ defer cancelDial()
+ dialer := &net.Dialer{}
+
+ var served int32
+ for n := attempts; n > 0; n-- {
wg.Add(1)
go func() {
defer wg.Done()
- c := http.Client{Timeout: 3 * time.Second}
- r, err := c.Get("http://" + l.Addr().String())
+
+ c, err := dialer.DialContext(dialCtx, l.Addr().Network(), l.Addr().String())
if err != nil {
t.Log(err)
- atomic.AddInt32(&failed, 1)
return
}
- defer r.Body.Close()
- io.Copy(ioutil.Discard, r.Body)
+ defer c.Close()
+
+ // Keep this end of the connection alive until after the Listener
+ // finishes.
+ conns := append(<-connc, c)
+ if len(conns) == max {
+ go func() {
+ // Give the server a bit of time to make sure it doesn't exceed its
+ // limit after serving this connection, then cancel the remaining
+ // Dials (if any).
+ time.Sleep(10 * time.Millisecond)
+ cancelDial()
+ l.Close()
+ }()
+ }
+ connc <- conns
+
+ b := make([]byte, len(msg))
+ if n, err := c.Read(b); n < len(b) {
+ t.Log(err)
+ return
+ }
+ atomic.AddInt32(&served, 1)
}()
}
wg.Wait()
- // We expect some Gets to fail as the kernel's accept queue is filled,
- // but most should succeed.
- if int(failed) >= attempts/2 {
- t.Errorf("%d requests failed within %d attempts", failed, attempts)
+ conns := <-connc
+ for _, c := range conns {
+ c.Close()
+ }
+ t.Logf("with limit %d, served %d connections (of %d dialed, %d attempted)", max, served, len(conns), attempts)
+ if served != max {
+ t.Errorf("expected exactly %d served", max)
}
}
@@ -87,27 +118,13 @@
// This used to hang.
func TestLimitListenerError(t *testing.T) {
- errCh := make(chan error, 1)
- go func() {
- defer close(errCh)
- const n = 2
- ll := LimitListener(errorListener{}, n)
- for i := 0; i < n+1; i++ {
- _, err := ll.Accept()
- if err != errFake {
- errCh <- fmt.Errorf("Accept error = %v; want errFake", err)
- return
- }
+ const n = 2
+ ll := LimitListener(errorListener{}, n)
+ for i := 0; i < n+1; i++ {
+ _, err := ll.Accept()
+ if err != errFake {
+ t.Fatalf("Accept error = %v; want errFake", err)
}
- }()
-
- select {
- case err := <-errCh:
- if err != nil {
- t.Fatalf("server: %v", err)
- }
- case <-time.After(timeout):
- t.Fatal("timeout. deadlock?")
}
}
@@ -122,7 +139,7 @@
errCh := make(chan error)
go func() {
defer close(errCh)
- c, err := net.DialTimeout("tcp", ln.Addr().String(), timeout)
+ c, err := net.Dial(ln.Addr().Network(), ln.Addr().String())
if err != nil {
errCh <- err
return
@@ -138,26 +155,21 @@
err = <-errCh
if err != nil {
- t.Fatalf("DialTimeout: %v", err)
+ t.Fatalf("Dial: %v", err)
}
- acceptDone := make(chan struct{})
- go func() {
- c, err := ln.Accept()
- if err == nil {
- c.Close()
- t.Errorf("Unexpected successful Accept()")
- }
- close(acceptDone)
- }()
+ // Allow the subsequent Accept to block before closing the listener.
+ // (Accept should unblock and return.)
+ timer := time.AfterFunc(10*time.Millisecond, func() {
+ ln.Close()
+ })
- // Wait a tiny bit to ensure the Accept() is blocking.
- time.Sleep(10 * time.Millisecond)
- ln.Close()
-
- select {
- case <-acceptDone:
- case <-time.After(timeout):
- t.Fatalf("Accept() still blocking")
+ c, err = ln.Accept()
+ if err == nil {
+ c.Close()
+ t.Errorf("Unexpected successful Accept()")
+ }
+ if timer.Stop() {
+ t.Errorf("Accept returned before listener closed: %v", err)
}
}
