Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http, x/net/http2: http.readTrackingBody data race after received GOAWAY frame #41234

Closed
answer1991 opened this issue Sep 5, 2020 · 6 comments

Comments

@answer1991
Copy link

@answer1991 answer1991 commented Sep 5, 2020

What version of Go are you using (go version)?

$ go version
go version go1.15 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/go/src/k8s.io/kubernetes/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build142705141=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Set up a HTTP2 server with a handler which will send GOAWAY frame
  2. Use a HTTP client to request HTTP2 server with POST method and data in multi goroutines in concurrency
  3. http.readTrackingBody cause data race

Example codes: https://github.com/answer1991/go-workspace/blob/master/pkg/goaway/goaway_test.go , run with go test -race failed:

WARNING: DATA RACE
Read at 0x00c00031ed38 by goroutine 181:
  bytes.(*Reader).Read()
      /usr/local/go/src/bytes/reader.go:41 +0x5e
  io/ioutil.(*nopCloser).Read()
      <autogenerated>:1 +0x87
  net/http.(*readTrackingBody).Read()
      /usr/local/go/src/net/http/transport.go:621 +0x8a
  golang.org/x/net/http2.(*clientStream).writeRequestBody()
      /Users/Joe/go/src/golang.org/x/net/http2/transport.go:1293 +0x761
  golang.org/x/net/http2.(*Transport).getBodyWriterState.func1()
      /Users/Joe/go/src/golang.org/x/net/http2/transport.go:2564 +0x13d

Previous write at 0x00c00031ed38 by goroutine 218:
  bytes.(*Reader).Read()
      /usr/local/go/src/bytes/reader.go:46 +0x14b
  io/ioutil.(*nopCloser).Read()
      <autogenerated>:1 +0x87
  net/http.(*readTrackingBody).Read()
      /usr/local/go/src/net/http/transport.go:621 +0x8a
  golang.org/x/net/http2.(*clientStream).writeRequestBody()
      /Users/Joe/go/src/golang.org/x/net/http2/transport.go:1293 +0x761
  golang.org/x/net/http2.(*Transport).getBodyWriterState.func1()
      /Users/Joe/go/src/golang.org/x/net/http2/transport.go:2564 +0x13d

Root Cause

Error timeline:

  1. http2 Transport read request body in a goroutine(let's named body reading goroutine):

https://github.com/golang/net/blob/62affa334b73ec65ed44a326519ac12c421905e3/http2/transport.go#L2630

  1. http2 Transport.RoundTrip return PROTOCOL_ERROR error after received GOAWAY frame, but the body reading goroutine is still on going and the request.Body(wrapped as http.readTrackingBody) has not been read.
  2. http Transport retry the request, found http.readTrackingBody.didRead == false, then re-use the http.readTrackingBody
  3. then there are 2 body reading goroutine try to read the same http.readTrackingBody instance.
@answer1991
Copy link
Author

@answer1991 answer1991 commented Sep 5, 2020

@answer1991 answer1991 changed the title net/http, x/net/http2: http.readTrackingBody cause data race after received GOAWAY frame net/http, x/net/http2: http.readTrackingBody data race after received GOAWAY frame Sep 5, 2020
@networkimprov
Copy link

@networkimprov networkimprov commented Sep 6, 2020

@fraenkel
Copy link
Contributor

@fraenkel fraenkel commented Sep 6, 2020

dupe of #31192

There are variations of this issue even when running this test case.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 7, 2020

Change https://golang.org/cl/253259 mentions this issue: net/http2: wait until the request body has been written

@answer1991
Copy link
Author

@answer1991 answer1991 commented Sep 7, 2020

@fraenkel thanks!

@andybons
Copy link
Member

@andybons andybons commented Sep 8, 2020

Duplicate of #31192

@andybons andybons marked this as a duplicate of #31192 Sep 8, 2020
@andybons andybons closed this Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.