src-d / hercules

Gaining advanced insights from Git repository history.
Other
2.63k stars 334 forks source link

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

Closed bobheadxi closed 5 years ago

bobheadxi commented 5 years ago

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

bobheadxi commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

It is 10pm here, so I will review this tomorrow :+1: