From 368f73bcd9651129c1753c3486cf5b0757d4707d Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 16 Oct 2015 16:28:42 +0000 Subject: [PATCH] net: unblock plan9 TCP Read calls after socket close Fixes #7782 Fixes #9554 Updates #7237 (original metabug, before we switched to specific bugs) Updates #11932 (plan9 still doesn't have net I/O deadline support) Change-Id: I96f311b88b1501d884ebc008fd31ad2cf1e16d75 Reviewed-on: https://go-review.googlesource.com/15941 Reviewed-by: Ian Lance Taylor Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/dial_test.go | 3 +++ src/net/fd_plan9.go | 8 +++++++ src/net/http/httputil/reverseproxy_test.go | 4 ---- src/net/http/serve_test.go | 11 +++++++-- src/net/http/transport_test.go | 27 ++++++++++++++-------- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/net/dial_test.go b/src/net/dial_test.go index 9843306775..bd3b2dd9b1 100644 --- a/src/net/dial_test.go +++ b/src/net/dial_test.go @@ -509,6 +509,9 @@ func TestDialerFallbackDelay(t *testing.T) { } func TestDialSerialAsyncSpuriousConnection(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip("skipping on plan9; no deadline support, golang.org/issue/11932") + } ln, err := newLocalListener("tcp") if err != nil { t.Fatal(err) diff --git a/src/net/fd_plan9.go b/src/net/fd_plan9.go index 32766f53b5..cec88609d0 100644 --- a/src/net/fd_plan9.go +++ b/src/net/fd_plan9.go @@ -171,6 +171,14 @@ func (fd *netFD) Close() error { if !fd.ok() { return syscall.EINVAL } + if fd.net == "tcp" { + // The following line is required to unblock Reads. + // For some reason, WriteString returns an error: + // "write /net/tcp/39/listen: inappropriate use of fd" + // But without it, Reads on dead conns hang forever. + // See Issue 9554. + fd.ctl.WriteString("hangup") + } err := fd.ctl.Close() if fd.data != nil { if err1 := fd.data.Close(); err1 != nil && err == nil { diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go index 14897f4118..1d309614e2 100644 --- a/src/net/http/httputil/reverseproxy_test.go +++ b/src/net/http/httputil/reverseproxy_test.go @@ -14,7 +14,6 @@ import ( "net/http/httptest" "net/url" "reflect" - "runtime" "strings" "testing" "time" @@ -226,9 +225,6 @@ func TestReverseProxyFlushInterval(t *testing.T) { } func TestReverseProxyCancelation(t *testing.T) { - if runtime.GOOS == "plan9" { - t.Skip("skipping test; see https://golang.org/issue/9554") - } const backendResponse = "I am the backend" reqInFlight := make(chan struct{}) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index dddfd40168..9def81af6b 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -26,6 +26,7 @@ import ( "os/exec" "reflect" "runtime" + "sort" "strconv" "strings" "sync" @@ -2980,6 +2981,7 @@ func TestServerConnState(t *testing.T) { if _, err := io.WriteString(c, "BOGUS REQUEST\r\n\r\n"); err != nil { t.Fatal(err) } + c.Read(make([]byte, 1)) // block until server hangs up on us c.Close() } @@ -3013,9 +3015,14 @@ func TestServerConnState(t *testing.T) { } logString := func(m map[int][]ConnState) string { var b bytes.Buffer - for id, l := range m { + var keys []int + for id := range m { + keys = append(keys, id) + } + sort.Ints(keys) + for _, id := range keys { fmt.Fprintf(&b, "Conn %d: ", id) - for _, s := range l { + for _, s := range m[id] { fmt.Fprintf(&b, "%s ", s) } b.WriteString("\n") diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index cc19342c30..5811650b0e 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -874,9 +874,6 @@ func TestTransportGzipShort(t *testing.T) { // tests that persistent goroutine connections shut down when no longer desired. func TestTransportPersistConnLeak(t *testing.T) { - if runtime.GOOS == "plan9" { - t.Skip("skipping test; see https://golang.org/issue/7237") - } defer afterTest(t) gotReqCh := make(chan bool) unblockCh := make(chan bool) @@ -943,9 +940,6 @@ func TestTransportPersistConnLeak(t *testing.T) { // golang.org/issue/4531: Transport leaks goroutines when // request.ContentLength is explicitly short func TestTransportPersistConnLeakShortBody(t *testing.T) { - if runtime.GOOS == "plan9" { - t.Skip("skipping test; see https://golang.org/issue/7237") - } defer afterTest(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { })) @@ -2291,15 +2285,28 @@ type errorReader struct { func (e errorReader) Read(p []byte) (int, error) { return 0, e.err } +type plan9SleepReader struct{} + +func (plan9SleepReader) Read(p []byte) (int, error) { + if runtime.GOOS == "plan9" { + // After the fix to unblock TCP Reads in + // https://golang.org/cl/15941, this sleep is required + // on plan9 to make sure TCP Writes before an + // immediate TCP close go out on the wire. On Plan 9, + // it seems that a hangup of a TCP connection with + // queued data doesn't send the queued data first. + // https://golang.org/issue/9554 + time.Sleep(50 * time.Millisecond) + } + return 0, io.EOF +} + type closerFunc func() error func (f closerFunc) Close() error { return f() } // Issue 6981 func TestTransportClosesBodyOnError(t *testing.T) { - if runtime.GOOS == "plan9" { - t.Skip("skipping test; see https://golang.org/issue/7782") - } defer afterTest(t) readBody := make(chan error, 1) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { @@ -2313,7 +2320,7 @@ func TestTransportClosesBodyOnError(t *testing.T) { io.Reader io.Closer }{ - io.MultiReader(io.LimitReader(neverEnding('x'), 1<<20), errorReader{fakeErr}), + io.MultiReader(io.LimitReader(neverEnding('x'), 1<<20), plan9SleepReader{}, errorReader{fakeErr}), closerFunc(func() error { select { case didClose <- true: