Commit 80e70a3f authored by Tom Bergan's avatar Tom Bergan

http2: fix race on ClientConn.maxFrameSize

This fixes TestTrailersClientToServer_h2. Before this CL, the following
command reliably fails. With this CL merged into net/http, the following
command reliably succeeds.

go test -race -run=TestTrailersClientToServer_h2 -count 1000 net/http

Updates golang/go#22721

Change-Id: I05d1504c60854fcf3ae9531f36a126e94b00f0b7
Reviewed-on: https://go-review.googlesource.com/79238Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
parent 3e050b2a
...@@ -811,7 +811,7 @@ func (cc *ClientConn) roundTrip(req *http.Request) (res *http.Response, gotErrAf ...@@ -811,7 +811,7 @@ func (cc *ClientConn) roundTrip(req *http.Request) (res *http.Response, gotErrAf
cc.wmu.Lock() cc.wmu.Lock()
endStream := !hasBody && !hasTrailers endStream := !hasBody && !hasTrailers
werr := cc.writeHeaders(cs.ID, endStream, hdrs) werr := cc.writeHeaders(cs.ID, endStream, int(cc.maxFrameSize), hdrs)
cc.wmu.Unlock() cc.wmu.Unlock()
traceWroteHeaders(cs.trace) traceWroteHeaders(cs.trace)
cc.mu.Unlock() cc.mu.Unlock()
...@@ -964,13 +964,12 @@ func (cc *ClientConn) awaitOpenSlotForRequest(req *http.Request) error { ...@@ -964,13 +964,12 @@ func (cc *ClientConn) awaitOpenSlotForRequest(req *http.Request) error {
} }
// requires cc.wmu be held // requires cc.wmu be held
func (cc *ClientConn) writeHeaders(streamID uint32, endStream bool, hdrs []byte) error { func (cc *ClientConn) writeHeaders(streamID uint32, endStream bool, maxFrameSize int, hdrs []byte) error {
first := true // first frame written (HEADERS is first, then CONTINUATION) first := true // first frame written (HEADERS is first, then CONTINUATION)
frameSize := int(cc.maxFrameSize)
for len(hdrs) > 0 && cc.werr == nil { for len(hdrs) > 0 && cc.werr == nil {
chunk := hdrs chunk := hdrs
if len(chunk) > frameSize { if len(chunk) > maxFrameSize {
chunk = chunk[:frameSize] chunk = chunk[:maxFrameSize]
} }
hdrs = hdrs[len(chunk):] hdrs = hdrs[len(chunk):]
endHeaders := len(hdrs) == 0 endHeaders := len(hdrs) == 0
...@@ -1087,13 +1086,17 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) ( ...@@ -1087,13 +1086,17 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
} }
} }
cc.mu.Lock()
maxFrameSize := int(cc.maxFrameSize)
cc.mu.Unlock()
cc.wmu.Lock() cc.wmu.Lock()
defer cc.wmu.Unlock() defer cc.wmu.Unlock()
// Two ways to send END_STREAM: either with trailers, or // Two ways to send END_STREAM: either with trailers, or
// with an empty DATA frame. // with an empty DATA frame.
if len(trls) > 0 { if len(trls) > 0 {
err = cc.writeHeaders(cs.ID, true, trls) err = cc.writeHeaders(cs.ID, true, maxFrameSize, trls)
} else { } else {
err = cc.fr.WriteData(cs.ID, true, nil) err = cc.fr.WriteData(cs.ID, true, nil)
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment