Skip to content

Commit

Permalink
net: unblock plan9 TCP Read calls after socket close
Browse files Browse the repository at this point in the history
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 <iant@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
bradfitz committed Oct 16, 2015
1 parent 7d86d57 commit 368f73b
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 16 deletions.
3 changes: 3 additions & 0 deletions src/net/dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions src/net/fd_plan9.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 0 additions & 4 deletions src/net/http/httputil/reverseproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"net/http/httptest"
"net/url"
"reflect"
"runtime"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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{})
Expand Down
11 changes: 9 additions & 2 deletions src/net/http/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"os/exec"
"reflect"
"runtime"
"sort"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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")
Expand Down
27 changes: 17 additions & 10 deletions src/net/http/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
}))
Expand Down Expand Up @@ -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) {
Expand All @@ -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:
Expand Down

0 comments on commit 368f73b

Please sign in to comment.