Skip to content

Commit 67fa797

Browse files
committed
net/http: update bundled http2; fixes TestTransportAndServerSharedBodyRace_h2
Update bundled http2 to git rev d1ba260648 (https://golang.org/cl/18288). Fixes the flaky TestTransportAndServerSharedBodyRace_h2. Also adds some debugging to TestTransportAndServerSharedBodyRace_h2 which I hope won't ever be necessary again, but I know will be. Fixes #13556 Change-Id: Ibcf2fc23ec0122dcac8891fdc3bd7f8acddd880e Reviewed-on: https://go-review.googlesource.com/18289 Reviewed-by: Andrew Gerrand <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent e9fc522 commit 67fa797

File tree

2 files changed

+95
-11
lines changed

2 files changed

+95
-11
lines changed

src/net/http/h2_bundle.go

Lines changed: 47 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/net/http/serve_test.go

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"os/exec"
2828
"reflect"
2929
"runtime"
30+
"runtime/debug"
3031
"sort"
3132
"strconv"
3233
"strings"
@@ -3038,19 +3039,47 @@ func TestTransportAndServerSharedBodyRace_h1(t *testing.T) {
30383039
testTransportAndServerSharedBodyRace(t, h1Mode)
30393040
}
30403041
func TestTransportAndServerSharedBodyRace_h2(t *testing.T) {
3041-
t.Skip("failing in http2 mode; golang.org/issue/13556")
30423042
testTransportAndServerSharedBodyRace(t, h2Mode)
30433043
}
30443044
func testTransportAndServerSharedBodyRace(t *testing.T, h2 bool) {
30453045
defer afterTest(t)
30463046

30473047
const bodySize = 1 << 20
30483048

3049+
// errorf is like t.Errorf, but also writes to println. When
3050+
// this test fails, it hangs. This helps debugging and I've
3051+
// added this enough times "temporarily". It now gets added
3052+
// full time.
3053+
errorf := func(format string, args ...interface{}) {
3054+
v := fmt.Sprintf(format, args...)
3055+
println(v)
3056+
t.Error(v)
3057+
}
3058+
30493059
unblockBackend := make(chan bool)
30503060
backend := newClientServerTest(t, h2, HandlerFunc(func(rw ResponseWriter, req *Request) {
3051-
io.CopyN(rw, req.Body, bodySize)
3061+
gone := rw.(CloseNotifier).CloseNotify()
3062+
didCopy := make(chan interface{})
3063+
go func() {
3064+
n, err := io.CopyN(rw, req.Body, bodySize)
3065+
didCopy <- []interface{}{n, err}
3066+
}()
3067+
isGone := false
3068+
Loop:
3069+
for {
3070+
select {
3071+
case <-didCopy:
3072+
break Loop
3073+
case <-gone:
3074+
isGone = true
3075+
case <-time.After(time.Second):
3076+
println("1 second passes in backend, proxygone=", isGone)
3077+
}
3078+
}
30523079
<-unblockBackend
30533080
}))
3081+
var quitTimer *time.Timer
3082+
defer func() { quitTimer.Stop() }()
30543083
defer backend.close()
30553084

30563085
backendRespc := make(chan *Response, 1)
@@ -3063,17 +3092,17 @@ func testTransportAndServerSharedBodyRace(t *testing.T, h2 bool) {
30633092

30643093
bresp, err := proxy.c.Do(req2)
30653094
if err != nil {
3066-
t.Errorf("Proxy outbound request: %v", err)
3095+
errorf("Proxy outbound request: %v", err)
30673096
return
30683097
}
30693098
_, err = io.CopyN(ioutil.Discard, bresp.Body, bodySize/2)
30703099
if err != nil {
3071-
t.Errorf("Proxy copy error: %v", err)
3100+
errorf("Proxy copy error: %v", err)
30723101
return
30733102
}
30743103
backendRespc <- bresp // to close later
30753104

3076-
// Try to cause a race: Both the DefaultTransport and the proxy handler's Server
3105+
// Try to cause a race: Both the Transport and the proxy handler's Server
30773106
// will try to read/close req.Body (aka req2.Body)
30783107
if h2 {
30793108
close(cancel)
@@ -3083,6 +3112,20 @@ func testTransportAndServerSharedBodyRace(t *testing.T, h2 bool) {
30833112
rw.Write([]byte("OK"))
30843113
}))
30853114
defer proxy.close()
3115+
defer func() {
3116+
// Before we shut down our two httptest.Servers, start a timer.
3117+
// We choose 7 seconds because httptest.Server starts logging
3118+
// warnings to stderr at 5 seconds. If we don't disarm this bomb
3119+
// in 7 seconds (after the two httptest.Server.Close calls above),
3120+
// then we explode with stacks.
3121+
quitTimer = time.AfterFunc(7*time.Second, func() {
3122+
debug.SetTraceback("ALL")
3123+
stacks := make([]byte, 1<<20)
3124+
stacks = stacks[:runtime.Stack(stacks, true)]
3125+
fmt.Fprintf(os.Stderr, "%s", stacks)
3126+
log.Fatalf("Timeout.")
3127+
})
3128+
}()
30863129

30873130
defer close(unblockBackend)
30883131
req, _ := NewRequest("POST", proxy.ts.URL, io.LimitReader(neverEnding('a'), bodySize))

0 commit comments

Comments
 (0)