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

Data race in new discovery test #3457

Closed
grobie opened this Issue Nov 12, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@grobie
Copy link
Member

grobie commented Nov 12, 2017

The new discovery test setup is not concurrancy safe. The atomic package must be used for all reads, not only for writing values.

==================
WARNING: DATA RACE
Read at 0x00c420453478 by goroutine 42:
  github.com/prometheus/prometheus/discovery.TestTargetSetThrottlesTheSyncCalls()
      /home/grobie/code/go/src/github.com/prometheus/prometheus/discovery/discovery_test.go:503 +0x6765
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:746 +0x16c

Previous write at 0x00c420453478 by goroutine 44:
  sync/atomic.AddInt32()
      /usr/lib/go/src/runtime/race_amd64.s:269 +0xb
  github.com/prometheus/prometheus/discovery.mockTargetProvider.Run()
      /home/grobie/code/go/src/github.com/prometheus/prometheus/discovery/discovery_test.go:1034 +0x40
  github.com/prometheus/prometheus/discovery.(*mockTargetProvider).Run()
      <autogenerated>:1 +0xa2

Goroutine 42 (running) created at:
  testing.(*T).Run()
      /usr/lib/go/src/testing/testing.go:789 +0x568
  testing.runTests.func1()
      /usr/lib/go/src/testing/testing.go:1004 +0xa7
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:746 +0x16c
  testing.runTests()
      /usr/lib/go/src/testing/testing.go:1002 +0x521
  testing.(*M).Run()
      /usr/lib/go/src/testing/testing.go:921 +0x206
  main.main()
      github.com/prometheus/prometheus/discovery/_test/_testmain.go:50 +0x1d3

Goroutine 44 (finished) created at:
  github.com/prometheus/prometheus/discovery.(*TargetSet).updateProviders()
      /home/grobie/code/go/src/github.com/prometheus/prometheus/discovery/discovery.go:250 +0x3aa
  github.com/prometheus/prometheus/discovery.(*TargetSet).Run()
      /home/grobie/code/go/src/github.com/prometheus/prometheus/discovery/discovery.go:199 +0x3bf
==================

@grandbora Can you fix this please? You can check for the existance of data races with go test -race ./discovery/....

@grandbora

This comment has been minimized.

Copy link
Contributor

grandbora commented Nov 12, 2017

On it.

@grandbora

This comment has been minimized.

Copy link
Contributor

grandbora commented Nov 13, 2017

#3458 fixes this issue. cc @grobie @brian-brazil

@AndreaGiardini

This comment has been minimized.

Copy link
Contributor

AndreaGiardini commented Dec 19, 2017

I think this issue can be closed

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 23, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 23, 2019

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.