Skip to content

Commit d3f88ce

Browse files
committed
net/http/httptest: close conns in StateNew on Server close
This part got dropped when we were debating between two solutions in https://golang.org/cl/15151 Fixes golang#13032 Change-Id: I820b94f6c0c102ccf9342abf957328ea01f49a26 Reviewed-on: https://go-review.googlesource.com/16313 Reviewed-by: Austin Clements <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent e243d24 commit d3f88ce

2 files changed

Lines changed: 49 additions & 1 deletion

File tree

src/net/http/httptest/server.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,25 @@ func (s *Server) Close() {
150150
s.Listener.Close()
151151
s.Config.SetKeepAlivesEnabled(false)
152152
for c, st := range s.conns {
153-
if st == http.StateIdle {
153+
// Force-close any idle connections (those between
154+
// requests) and new connections (those which connected
155+
// but never sent a request). StateNew connections are
156+
// super rare and have only been seen (in
157+
// previously-flaky tests) in the case of
158+
// socket-late-binding races from the http Client
159+
// dialing this server and then getting an idle
160+
// connection before the dial completed. There is thus
161+
// a connected connection in StateNew with no
162+
// associated Request. We only close StateIdle and
163+
// StateNew because they're not doing anything. It's
164+
// possible StateNew is about to do something in a few
165+
// milliseconds, but a previous CL to check again in a
166+
// few milliseconds wasn't liked (early versions of
167+
// https://golang.org/cl/15151) so now we just
168+
// forcefully close StateNew. The docs for Server.Close say
169+
// we wait for "oustanding requests", so we don't close things
170+
// in StateActive.
171+
if st == http.StateIdle || st == http.StateNew {
154172
s.closeConn(c)
155173
}
156174
}

src/net/http/httptest/server_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
package httptest
66

77
import (
8+
"bufio"
89
"io/ioutil"
10+
"net"
911
"net/http"
1012
"testing"
1113
)
@@ -54,3 +56,31 @@ func TestGetAfterClose(t *testing.T) {
5456
t.Fatalf("Unexected response after close: %v, %v, %s", res.Status, res.Header, body)
5557
}
5658
}
59+
60+
func TestServerCloseBlocking(t *testing.T) {
61+
ts := NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
62+
w.Write([]byte("hello"))
63+
}))
64+
dial := func() net.Conn {
65+
c, err := net.Dial("tcp", ts.Listener.Addr().String())
66+
if err != nil {
67+
t.Fatal(err)
68+
}
69+
return c
70+
}
71+
72+
// Keep one connection in StateNew (connected, but not sending anything)
73+
cnew := dial()
74+
defer cnew.Close()
75+
76+
// Keep one connection in StateIdle (idle after a request)
77+
cidle := dial()
78+
defer cidle.Close()
79+
cidle.Write([]byte("HEAD / HTTP/1.1\r\nHost: foo\r\n\r\n"))
80+
_, err := http.ReadResponse(bufio.NewReader(cidle), nil)
81+
if err != nil {
82+
t.Fatal(err)
83+
}
84+
85+
ts.Close() // test we don't hang here forever.
86+
}

0 commit comments

Comments
 (0)