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

x/net/http2: data race in TestCanonicalHeaderCacheGrowth #57218

Closed
bcmills opened this issue Dec 9, 2022 · 3 comments
Closed

x/net/http2: data race in TestCanonicalHeaderCacheGrowth #57218

bcmills opened this issue Dec 9, 2022 · 3 comments
Assignees
@neild
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.

Comments

@bcmills
Copy link
Contributor

bcmills commented Dec 9, 2022 

#!watchflakes
post <- pkg == "x/net/http2" && test == "TestCanonicalHeaderCacheGrowth" && `DATA RACE`

https://build.golang.org/log/9f76836a9c3da1bd2d60e0e4b5274e6d253febaa:

==================
WARNING: DATA RACE
Write at 0x000001a1e0cc by goroutine 67:
  golang.org/x/net/http2.disableGoroutineTracking()
      C:/workdir/gopath/src/golang.org/x/net/http2/server_test.go:3502 +0x64
  golang.org/x/net/http2.TestCanonicalHeaderCacheGrowth()
      C:/workdir/gopath/src/golang.org/x/net/http2/server_test.go:4550 +0x9b
  testing.tRunner()
      C:/workdir/go/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      C:/workdir/go/src/testing/testing.go:1486 +0x47

Previous read at 0x000001a1e0cc by goroutine 248:
  golang.org/x/net/http2.goroutineLock.checkNotOn()
      C:/workdir/gopath/src/golang.org/x/net/http2/gotrack.go:41 +0xaa
  golang.org/x/net/http2.(*serverConn).writeFrameFromHandler()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:1134 +0x75
  golang.org/x/net/http2.(*serverConn).writeHeaders()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2331 +0x116
  golang.org/x/net/http2.(*responseWriterState).writeChunk()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2617 +0x10f2
  golang.org/x/net/http2.chunkWriter.Write()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2520 +0xb5
  golang.org/x/net/http2.(*responseWriter).FlushError()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2790 +0xbb
  golang.org/x/net/http2.(*responseWriter).Flush()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2774 +0x78
  golang.org/x/net/http2.(*responseWriter).handlerDone()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2952 +0x6e
  golang.org/x/net/http2.(*serverConn).runHandler.func1()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2303 +0x3de
  runtime.deferreturn()
      C:/workdir/go/src/runtime/panic.go:436 +0x32
  golang.org/x/net/http2.(*serverConn).processHeaders.func1()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2018 +0x64

Goroutine 67 (running) created at:
  testing.(*T).Run()
      C:/workdir/go/src/testing/testing.go:1486 +0x724
  testing.runTests.func1()
      C:/workdir/go/src/testing/testing.go:1839 +0x99
  testing.tRunner()
      C:/workdir/go/src/testing/testing.go:1439 +0x213
  testing.runTests()
      C:/workdir/go/src/testing/testing.go:1837 +0x7e4
  testing.(*M).Run()
      C:/workdir/go/src/testing/testing.go:1719 +0xa71
  main.main()
      _testmain.go:767 +0x2e4

Goroutine 248 (finished) created at:
  golang.org/x/net/http2.(*serverConn).processHeaders()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:2018 +0xd39
  golang.org/x/net/http2.(*serverConn).processFrame()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:1518 +0x635
  golang.org/x/net/http2.(*serverConn).processFrameFromReader()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:1461 +0x2ed
  golang.org/x/net/http2.(*serverConn).serve()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:952 +0x15ea
  golang.org/x/net/http2.(*Server).ServeConn()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:531 +0x1771
  golang.org/x/net/http2.ConfigureServer.func1()
      C:/workdir/gopath/src/golang.org/x/net/http2/server.go:321 +0x124
  net/http.(*conn).serve()
      C:/workdir/go/src/net/http/server.go:1874 +0x1c56
  net/http.(*Server).Serve.func3()
      C:/workdir/go/src/net/http/server.go:3071 +0x58
==================
--- FAIL: TestCanonicalHeaderCacheGrowth (86.60s)
    testing.go:1312: race detected during execution of test
FAIL
FAIL	golang.org/x/net/http2	111.467s
@bcmills
Copy link
Contributor Author

bcmills commented Dec 9, 2022

This is the regression test added for #56350 in CL 455635 (attn @neild).

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 9, 2022
@dmitshur
Copy link
Contributor

Damien investigated this problem in the backports, and found the fix was to set serveG (see here). The same fix just needs to be applied to the main branch too.

@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 10, 2022
@gopherbot
Copy link

Change https://go.dev/cl/456523 mentions this issue: http2: fix race in TestCanonicalHeaderCacheGrowth

@golang golang locked and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@neild neild

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Test Flakes
Status: Done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@neild @dmitshur @bcmills @gopherbot