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/pprof: data race in StartCPUProfile #8365

Closed
gopherbot opened this issue Jul 13, 2014 · 3 comments
Closed

net/http/pprof: data race in StartCPUProfile #8365

gopherbot opened this issue Jul 13, 2014 · 3 comments
Milestone

Comments

@gopherbot
Copy link

by beauze.h:

Hi,

While trying to profile a program (compiled with -race) the data race pasted below
appeared:
The warning appeared while refreshing a few times in a row the following page:
(http://localhost:6060/debug/pprof/profile)

I'm still new to go, so if this is just a chair/keyboard problem, or not a problem at
all, please accept my apologies :)

go version is go1.3 linux/amd64

==================
WARNING: DATA RACE
Write by goroutine 23:
  runtime/pprof.StopCPUProfile()
      /usr/lib/go/src/pkg/runtime/pprof/pprof.go:619 +0xa6
  net/http/pprof.Profile()
      /usr/lib/go/src/pkg/net/http/pprof/pprof.go:98 +0x2e9
  net/http.HandlerFunc.ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1235 +0x4d
  net/http.(*ServeMux).ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1511 +0x21c
  net/http.serverHandler.ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1673 +0x1fc
  net/http.(*conn).serve()
      /usr/lib/go/src/pkg/net/http/server.go:1174 +0xf9e

Previous read by goroutine 210:
  runtime/pprof.StartCPUProfile()
      /usr/lib/go/src/pkg/runtime/pprof/pprof.go:579 +0x4f
  net/http/pprof.Profile()
      /usr/lib/go/src/pkg/net/http/pprof/pprof.go:88 +0x13b
  net/http.HandlerFunc.ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1235 +0x4d
  net/http.(*ServeMux).ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1511 +0x21c
  net/http.serverHandler.ServeHTTP()
      /usr/lib/go/src/pkg/net/http/server.go:1673 +0x1fc
  net/http.(*conn).serve()
      /usr/lib/go/src/pkg/net/http/server.go:1174 +0xf9e
  runtime.gosched0()
      /usr/lib/go/src/pkg/runtime/proc.c:1436 +0xaf
  crypto/tls.(*halfConn).decrypt()
      /usr/lib/go/src/pkg/crypto/tls/conn.go:284 +0xd97
  crypto/tls.(*Conn).readRecord()
      /usr/lib/go/src/pkg/crypto/tls/conn.go:595 +0x1154
  crypto/tls.(*Conn).readHandshake()
      /usr/lib/go/src/pkg/crypto/tls/conn.go:776 +0x14e
  crypto/tls.(*clientHandshakeState).readFinished()
      /usr/lib/go/src/pkg/crypto/tls/handshake_client.go:508 +0xf0
  crypto/tls.(*Conn).clientHandshake()
      /usr/lib/go/src/pkg/crypto/tls/handshake_client.go:196 +0x20c2
  crypto/tls.(*Conn).Handshake()
      /usr/lib/go/src/pkg/crypto/tls/conn.go:974 +0x144
  net/http.func·021()
      /usr/lib/go/src/pkg/net/http/transport.go:577 +0x5b

Goroutine 23 (running) created at:
  net/http.(*Server).Serve()
      /usr/lib/go/src/pkg/net/http/server.go:1721 +0x35d
  net/http.(*Server).ListenAndServe()
      /usr/lib/go/src/pkg/net/http/server.go:1688 +0x181
  net/http.ListenAndServe()
      /usr/lib/go/src/pkg/net/http/server.go:1778 +0xc3
  main.func·003()
      /home/chouquette/dev/go/src/worker/main.go:50 +0x4c

Goroutine 210 (running) created at:
  net/http.(*Server).Serve()
      /usr/lib/go/src/pkg/net/http/server.go:1721 +0x35d
  net/http.(*Server).ListenAndServe()
      /usr/lib/go/src/pkg/net/http/server.go:1688 +0x181
  net/http.ListenAndServe()
      /usr/lib/go/src/pkg/net/http/server.go:1778 +0xc3
  main.func·003()
      /home/chouquette/dev/go/src/worker/main.go:50 +0x4c
@dvyukov
Copy link
Member

dvyukov commented Jul 13, 2014

Comment 1:

Thanks for the report!
It's unclear why we do this double-checked locking in StartCPUProfile:
        // Avoid queueing behind StopCPUProfile.
        // Could use TryLock instead if we had it.
        if cpu.profiling {
                return fmt.Errorf("cpu profiling already in use")
        }
The comment seems to be wrong -- cpu.Lock is not held for the whole duration of
profiling. So we can just lock it and check cpu.profiling flag.

Labels changed: added release-go1.4, repo-main, threadsanitizer.

Owner changed to @dvyukov.

Status changed to Accepted.

@gopherbot
Copy link
Author

Comment 3:

CL https://golang.org/cl/116290043 mentions this issue.

@dvyukov
Copy link
Member

dvyukov commented Aug 12, 2014

Comment 4:

This issue was closed by revision fe7b29f.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
It's unclear why we do this broken double-checked locking.
The mutex is not held for the whole duration of CPU profiling.
Fixes golang#8365.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/116290043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
It's unclear why we do this broken double-checked locking.
The mutex is not held for the whole duration of CPU profiling.
Fixes golang#8365.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/116290043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants