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

Jump to bottom

net/http: data race during TestTrailersClientToServer_h2 #22721

Closed
ALTree opened this issue Nov 14, 2017 · 3 comments
Closed

net/http: data race during TestTrailersClientToServer_h2 #22721

ALTree opened this issue Nov 14, 2017 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.10

Comments

@ALTree
Copy link
Member

ALTree commented Nov 14, 2017

TestTrailersClientToServer_h2 triggers a data race. It was caught by a race builder here: https://build.golang.org/log/ede024321618b97ce309d1b255b936493545bc50

I can easily reproduce it on tip doing this:

$ cd /src/net/http
$ gotip test -race -run=TestTrailersClientToServer_h2 -count 500

==================
WARNING: DATA RACE
Write at 0x00c42015e0f0 by goroutine 37:
  net/http.(*http2clientConnReadLoop).processSettings.func1()
      /home/alberto/go/src/net/http/h2_bundle.go:8497 +0x3cb
  net/http.(*http2SettingsFrame).ForeachSetting()
      /home/alberto/go/src/net/http/h2_bundle.go:2038 +0x171
  net/http.(*http2clientConnReadLoop).processSettings()
      /home/alberto/go/src/net/http/h2_bundle.go:8494 +0x196
  net/http.(*http2clientConnReadLoop).run()
      /home/alberto/go/src/net/http/h2_bundle.go:8040 +0x6c5
  net/http.(*http2ClientConn).readLoop()
      /home/alberto/go/src/net/http/h2_bundle.go:7922 +0xde

Previous read at 0x00c42015e0f0 by goroutine 45:
  net/http.(*http2ClientConn).writeHeaders()
      /home/alberto/go/src/net/http/h2_bundle.go:7503 +0x57
  net/http.(*http2clientStream).writeRequestBody()
      /home/alberto/go/src/net/http/h2_bundle.go:7630 +0x81e
  net/http.(*http2Transport).getBodyWriterState.func1()
      /home/alberto/go/src/net/http/h2_bundle.go:8758 +0x129

Goroutine 37 (running) created at:
  net/http.(*http2Transport).newClientConn()
      /home/alberto/go/src/net/http/h2_bundle.go:7106 +0xd49
  net/http.(*http2Transport).NewClientConn()
      /home/alberto/go/src/net/http/h2_bundle.go:7043 +0x55
  net/http.(*http2addConnCall).run()
      /home/alberto/go/src/net/http/h2_bundle.go:836 +0x55

Goroutine 45 (running) created at:
  net/http.http2bodyWriterState.scheduleBodyWrite()
      /home/alberto/go/src/net/http/h2_bundle.go:8805 +0xb2
  net/http.(*http2ClientConn).RoundTrip()
      /home/alberto/go/src/net/http/h2_bundle.go:7361 +0x71d
  net/http.(*http2Transport).RoundTripOpt()
      /home/alberto/go/src/net/http/h2_bundle.go:6877 +0x3b7
  net/http.(*http2Transport).RoundTrip()
      /home/alberto/go/src/net/http/h2_bundle.go:6839 +0x4b
  net/http.(*Transport).RoundTrip()
      /home/alberto/go/src/net/http/transport.go:411 +0x986
  net/http.send()
      /home/alberto/go/src/net/http/client.go:252 +0x352
  net/http.(*Client).send()
      /home/alberto/go/src/net/http/client.go:176 +0x1b0
  net/http.(*Client).Do()
      /home/alberto/go/src/net/http/client.go:615 +0x4dd
  net/http_test.testTrailersClientToServer()
      /home/alberto/go/src/net/http/clientserver_test.go:611 +0x630
  net/http_test.TestTrailersClientToServer_h2()
      /home/alberto/go/src/net/http/clientserver_test.go:567 +0x3d
  testing.tRunner()
      /home/alberto/go/src/testing/testing.go:769 +0x169
==================
--- FAIL: TestTrailersClientToServer_h2 (0.01s)
	testing.go:722: race detected during execution of test
FAIL
exit status 1
FAIL	net/http	7.988s

We also have #22678 but the reported race seems different; may be a different bug.
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 14, 2017
@ALTree ALTree added this to the Go1.10 milestone Nov 14, 2017
@odeke-em
Copy link
Member

I too can reproduce this on tip, thanks for the repro @ALTree.

@tombergan tombergan self-assigned this Nov 21, 2017
@gopherbot
Copy link

Change https://golang.org/cl/79238 mentions this issue: http2: fix race on ClientConn.maxFrameSize

gopherbot pushed a commit to golang/net that referenced this issue Nov 27, 2017
@tombergan
http2: fix race on ClientConn.maxFrameSize
80e70a3 
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/79238
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/80078 mentions this issue: net/http: update bundled http2

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@tombergan
http2: fix race on ClientConn.maxFrameSize
e1e77be 
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/79238
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
jasonwbarnett pushed a commit to jasonwbarnett/fileserver that referenced this issue Jul 11, 2018
@bradfitz
net/http: update bundled http2
c4bdfb1 
Update http2 to x/net git rev db473f6b23.

(And un-skip TestWriteHeader0_h2 added in CL 80077, now fixed.)

Includes:

   http2: remove afterReqBodyWriteError wrapper
   https://golang.org/cl/75252

   http2: fix transport data race on reused *http.Request objects
   https://golang.org/cl/75530

   http2: require either ECDSA or RSA ciphersuite
   https://golang.org/cl/30721

   http2: don't log about timeouts reading client preface on new connections
   https://golang.org/cl/79498

   http2: don't crash in Transport on server's DATA following bogus HEADERS
   https://golang.org/cl/80056

   http2: panic on invalid WriteHeader status code
   https://golang.org/cl/80076

   http2: fix race on ClientConn.maxFrameSize
   https://golang.org/cl/79238

   http2: don't autodetect Content-Type when the response has an empty body
   https://golang.org/cl/80135

Fixes golang/go#18776
Updates golang/go#20784
Fixes golang/go#21316
Fixes golang/go#22721
Fixes golang/go#22880

Change-Id: Ie86e24e0ee2582a5a82afe5de3c7c801528be069
Reviewed-on: https://go-review.googlesource.com/80078
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
@golang golang locked and limited conversation to collaborators Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
4 participants
@ALTree @odeke-em @gopherbot @tombergan