net/http: deflake TestServerShutdownStateNew

This function tests that calling Shutdown on a Server that has a "new"
connection yet to write any bytes, in which case it should wait for five
seconds until considering the connection as "idle".

However, the test was flaky. If Shutdown happened to run before the
server accepted the connection, the connection would immediately be
rejected as the server is already closed, as opposed to being accepted
in the "new" state. Then, Shutdown would return almost immediately, as
it had no connections to wait for:

	--- FAIL: TestServerShutdownStateNew (2.00s)
	    serve_test.go:5603: shutdown too soon after 49.41µs
	    serve_test.go:5617: timeout waiting for Read to unblock

Fix this by making sure that the connection has been accepted before
calling Shutdown. Verified that the flake is gone after 50k concurrent
runs of the test with no failures, whereas the test used to fail around
10% of the time on my laptop:

	go test -c && stress -p 256 ./http.test -test.run TestServerShutdownStateNew

Fixes #26233.

Change-Id: I819d7eedb67c48839313427675facb39d9c17257
Reviewed-on: https://go-review.googlesource.com/122355
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index e597ac35a48580fd08da01e7775f58bfc0df860f..3624160a9932d48aec83be51e633799b71210e25 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -5569,18 +5569,32 @@ func TestServerShutdownStateNew(t *testing.T) {
        setParallel(t)
        defer afterTest(t)
 
-       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+       ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, r *Request) {
                // nothing.
        }))
+       var connAccepted sync.WaitGroup
+       ts.Config.ConnState = func(conn net.Conn, state ConnState) {
+               if state == StateNew {
+                       connAccepted.Done()
+               }
+       }
+       ts.Start()
        defer ts.Close()
 
        // Start a connection but never write to it.
+       connAccepted.Add(1)
        c, err := net.Dial("tcp", ts.Listener.Addr().String())
        if err != nil {
                t.Fatal(err)
        }
        defer c.Close()
 
+       // Wait for the connection to be accepted by the server. Otherwise, if
+       // Shutdown happens to run first, the server will be closed when
+       // encountering the connection, in which case it will be rejected
+       // immediately.
+       connAccepted.Wait()
+
        shutdownRes := make(chan error, 1)
        go func() {
                shutdownRes <- ts.Config.Shutdown(context.Background())