Enable race detection on test run, fix race condition by bobheadxi · Pull Request #232 · src-d/hercules

bobheadxi · 2019-03-07T19:15:44Z

Please feel free to close this if avoiding race conditions aren't a priority, but I noticed my -race-enabled tests would fail on what seemed like hercules code.

If the build for this PR fails (it does locally on my machine) I'll open a ticket about resolving concurrency issues.

Here's what I get when I run hercules' tests on my machine:

WARNING: DATA RACE
Write at 0x00c0001f2928 by goroutine 42:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func1.1()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:215 +0x38
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func1()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:267 +0x1036
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func3()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:327 +0x38

Previous write at 0x00c0001f2928 by goroutine 43:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func2.1()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:271 +0x38
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func2()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:322 +0x103b
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func4()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x38

Goroutine 42 (running) created at:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:327 +0x2526
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163

Goroutine 43 (finished) created at:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x2558
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163
==================
==================
WARNING: DATA RACE
Write at 0x00c00015d080 by goroutine 42:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func3()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:327 +0x59

Previous write at 0x00c00015d080 by goroutine 43:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func4()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x59

Goroutine 42 (running) created at:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:327 +0x2526
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163

Goroutine 43 (finished) created at:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x2558
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163
==================
==================
WARNING: DATA RACE
Read at 0x00c00015d080 by goroutine 41:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:330 +0x257a
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163

Previous write at 0x00c00015d080 by goroutine 43:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume.func4()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x59

Goroutine 41 (running) created at:
  testing.(*T).Run()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:916 +0x699
  testing.runTests.func1()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:1157 +0xa8
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163
  testing.runTests()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:1155 +0x523
  testing.(*M).Run()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:1072 +0x2eb
  main.main()
      _testmain.go:170 +0x222

Goroutine 43 (finished) created at:
  gopkg.in/src-d/hercules.v9/internal/plumbing.(*RenameAnalysis).Consume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames.go:328 +0x2558
  gopkg.in/src-d/hercules.v9/internal/plumbing.TestRenameAnalysisConsume()
      /Users/robertlin/go/src/gopkg.in/src-d/hercules.v9/internal/plumbing/renames_test.go:126 +0xb26
  testing.tRunner()
      /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0x163
==================

update: travis ran into the same issue, so I've opened #233 with some more details

update 2: working on resolving race conditions, so hopefully this will close #233 as well

Signed-off-by: Robert Lin <robertlin1@gmail.com>

bobheadxi · 2019-03-07T19:17:37Z

  - golint -set_exit_status $(go list ./... | grep -v /vendor/)
  - flake8
  - go test -coverpkg=all -v -coverprofile=coverage.txt -covermode=count gopkg.in/src-d/hercules.v9/... && sed -i '/cmd\/hercules\|core.go/d' coverage.txt
+  - go test -race gopkg.in/src-d/hercules.v9/... # run race test separately to preserve -covermode=count


note on this: it appears -race must be run with -covermode=atomic:

❯ go test -race -coverpkg=all -v -coverprofile=coverage.txt -covermode=count gopkg.in/src-d/hercules.v9/... && sed -i '/cmd\/hercules\|core.go/d' coverage.txt -covermode must be "atomic", not "count", when -race is enabled

It's not great having the test suite run twice though, so I'm holding off on updating the appveyor config for now, but imo having the race detection is much more valuable than -covermode=count, let me know what you think and I can swap it over entirely to -covermode=atomic.

I am fine with -covermode=atomic as long as it does not break the profile stats, so yeah, let's swap.

…aces Signed-off-by: Robert Lin <robertlin1@gmail.com>

bobheadxi · 2019-03-07T20:00:06Z

Not sure what happened with one of the build jobs: https://travis-ci.com/src-d/hercules/jobs/183188633

but the -race tests are passing in https://travis-ci.com/src-d/hercules/jobs/183188632

vmarkovtsev · 2019-03-07T20:43:57Z

Hi @bobheadxi thanks so much for your contribution. That build failed because of an annoying problem in Travis environment related to networking:

grouped write of manifest, lock and vendor: error while writing out vendor tree: failed to write dep tree: failed to export golang.org/x/sys: failed to fetch source for https://go.googlesource.com/sys: unable to get repository: Cloning into '/home/travis/gopath/pkg/dep/sources/https---go.googlesource.com-sys'...
fatal: unable to access 'https://go.googlesource.com/sys/': The requested URL returned error: 502

I restarted that job. I have recently had to restart it 5 times before that error goes away, which is very annoying. We are thinking with @smola what can be done to resolve it.

vmarkovtsev · 2019-03-07T20:46:34Z

It is 10pm here, so I will review this tomorrow 👍

vmarkovtsev · 2019-03-08T09:06:25Z

  - golint -set_exit_status $(go list ./... | grep -v /vendor/)
  - flake8
  - go test -coverpkg=all -v -coverprofile=coverage.txt -covermode=count gopkg.in/src-d/hercules.v9/... && sed -i '/cmd\/hercules\|core.go/d' coverage.txt
+  - go test -race gopkg.in/src-d/hercules.v9/... # run race test separately to preserve -covermode=count


I am fine with -covermode=atomic as long as it does not break the profile stats, so yeah, let's swap.

Enable race detection on test run, fix race condition

bobheadxi added 2 commits March 7, 2019 11:14

Enable race detection on test run

60eb49c

Signed-off-by: Robert Lin <robertlin1@gmail.com>

Add coverage.txt to gitignore

c577214

Signed-off-by: Robert Lin <robertlin1@gmail.com>

bobheadxi commented Mar 7, 2019

View reviewed changes

bobheadxi mentioned this pull request Mar 7, 2019

Resolve concurrency issues #233

Closed

Use channel communication in RenameAnalysis.Consume to resolve data r…

e4d6e7a

…aces Signed-off-by: Robert Lin <robertlin1@gmail.com>

bobheadxi changed the title ~~Enable race detection on test run~~ Enable race detection on test run, fix race condition Mar 7, 2019

vmarkovtsev approved these changes Mar 8, 2019

View reviewed changes

vmarkovtsev merged commit d6c5828 into src-d:master Mar 8, 2019

dmytrogajewski pushed a commit to dmytrogajewski/hercules that referenced this pull request Jul 28, 2025

Merge pull request src-d#232 from bobheadxi/master

80bb04a

Enable race detection on test run, fix race condition

CWBudde pushed a commit to CWBudde/hercules that referenced this pull request Apr 10, 2026

Merge pull request src-d#232 from bobheadxi/master

9ffef8a

Enable race detection on test run, fix race condition

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable race detection on test run, fix race condition#232

Enable race detection on test run, fix race condition#232
vmarkovtsev merged 3 commits into
src-d:masterfrom
bobheadxi:master

bobheadxi commented Mar 7, 2019 •

edited

Loading

Uh oh!

bobheadxi Mar 7, 2019 •

edited

Loading

Uh oh!

vmarkovtsev Mar 8, 2019

Uh oh!

bobheadxi commented Mar 7, 2019

Uh oh!

vmarkovtsev commented Mar 7, 2019

Uh oh!

vmarkovtsev commented Mar 7, 2019

Uh oh!

vmarkovtsev Mar 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bobheadxi commented Mar 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bobheadxi Mar 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vmarkovtsev Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

bobheadxi commented Mar 7, 2019

Uh oh!

vmarkovtsev commented Mar 7, 2019

Uh oh!

vmarkovtsev commented Mar 7, 2019

Uh oh!

vmarkovtsev Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bobheadxi commented Mar 7, 2019 •

edited

Loading

bobheadxi Mar 7, 2019 •

edited

Loading