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

testing: `go -cover -race` finds race in coverage code @ tip #8630

Closed
lavalamp opened this issue Sep 2, 2014 · 8 comments

Comments

Projects
None yet
7 participants
@lavalamp
Copy link

commented Sep 2, 2014

$ go version
go version devel +a1d74ad863fb Tue Sep 02 12:23:49 2014 -0700 linux/amd64

One sample race:

==================
WARNING: DATA RACE
Read by main goroutine:
  testing.coverReport()
      /home/travis/.gvm/gos/tip/src/pkg/testing/cover.go:89 +0x4f8
  testing.after()
      /home/travis/.gvm/gos/tip/src/pkg/testing/testing.go:596 +0x89d
  testing.Main()
      /home/travis/.gvm/gos/tip/src/pkg/testing/testing.go:450 +0x1e1
  main.main()
      github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config/_test/_testmain.go:117 +0x17a
Previous write by goroutine 11:
  sync/atomic.AddInt32()
      /home/travis/.gvm/gos/tip/src/pkg/runtime/race_amd64.s:255 +0xc
  github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config.handleServicesWatch()
      github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config/_test/_obj_test/api.go:93 +0x261
  github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config.(*SourceAPI).runServices()
      github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config/_test/_obj_test/api.go:71 +0x44b
  github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config.func·007()
      /home/travis/gopath/src/github.com/GoogleCloudPlatform/kubernetes/_output/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config/api_test.go:39 +0x5e
Goroutine 11 (running) created at:
  github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config.TestServices()
      /home/travis/gopath/src/github.com/GoogleCloudPlatform/kubernetes/_output/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config/api_test.go:40 +0x377
  testing.tRunner()
      /home/travis/.gvm/gos/tip/src/pkg/testing/testing.go:427 +0x112
==================
@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 2, 2014

Comment 1:

Why are you filing a bug against Go instead of a bug against Kubernetes?
@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 2, 2014

Comment 2:

Oh, you're saying the generated code from -cover has data races? Can you confirm that
there's no race when you run with -race and without -cover?
@lavalamp

This comment has been minimized.

Copy link
Author

commented Sep 3, 2014

Comment 3:

Right, the coverage code has a race (or a false positive).
Yes, I can confirm that turning off coverage (and nothing else) causes the races to go
away: https://travis-ci.org/GoogleCloudPlatform/kubernetes/jobs/34241151
@adg

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2014

Comment 4:

What if you set -covermode=atomic ?
@adg

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2014

Comment 5:

Oh, I see from the stack trace that covermode _is_ set to atomic. (the write by
atomic.AddInt32 is the giveaway)

Labels changed: added release-go1.4.

Status changed to Accepted.

@dvyukov

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

Comment 6:

Counters reading is not atomic. It must be.

Labels changed: added repo-main, threadsanitizer.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 8, 2014

Comment 7:

CL https://golang.org/cl/141800043 mentions this issue.
@robpike

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2014

Comment 8:

This issue was closed by revision d33ee0c.

Status changed to Fixed.

@lavalamp lavalamp added fixed labels Sep 9, 2014

@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 added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018

testing: read coverage counters atomically
For -mode=atomic, we need to read the counters
using an atomic load to avoid a race. Not worth worrying
about when -mode=atomic is set during generation
of the profile, so we use atomic loads always.

Fixes golang#8630.

LGTM=rsc
R=dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/141800043

wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018

testing: read coverage counters atomically
For -mode=atomic, we need to read the counters
using an atomic load to avoid a race. Not worth worrying
about when -mode=atomic is set during generation
of the profile, so we use atomic loads always.

Fixes golang#8630.

LGTM=rsc
R=dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/141800043

wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018

testing: read coverage counters atomically
For -mode=atomic, we need to read the counters
using an atomic load to avoid a race. Not worth worrying
about when -mode=atomic is set during generation
of the profile, so we use atomic loads always.

Fixes golang#8630.

LGTM=rsc
R=dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/141800043

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.