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

x/tools/go/loader: data race reported while running TestCycles #36415

Closed
dmitshur opened this issue Jan 6, 2020 · 5 comments
Closed

x/tools/go/loader: data race reported while running TestCycles #36415

dmitshur opened this issue Jan 6, 2020 · 5 comments

Comments

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Jan 6, 2020

From https://build.golang.org/log/1667c1c432e0389a87b0b5433d895b42eccd7716, which is a linux-amd64-race test run of golang/tools@774c71f using Go tip (1.14-ish):

[...]
==================
WARNING: DATA RACE
Read at 0x00c00713ff20 by goroutine 41:
  go/types.(*Package).Imports()
      /workdir/go/src/go/types/package.go:56 +0x2a1
  golang.org/x/tools/go/loader.markErrorFreePackages()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:686 +0x12f
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:669 +0x1d31
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb

Previous write at 0x00c00713ff20 by goroutine 159:
  go/types.(*Checker).collectObjects()
      /workdir/go/src/go/types/resolver.go:264 +0x1c8a
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:255 +0xd7
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:248 +0x2a2
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1030 +0x24c
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 41 (running) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1005 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1247 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1245 +0x527
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1162 +0x2ff
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223

Goroutine 159 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46
==================
==================
WARNING: DATA RACE
Read at 0x00c004d53228 by goroutine 41:
  golang.org/x/tools/go/loader.markErrorFreePackages()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:686 +0x161
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:669 +0x1d31
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb

Previous write at 0x00c004d53228 by goroutine 159:
  go/types.(*Checker).collectObjects()
      /workdir/go/src/go/types/resolver.go:264 +0x1c50
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:255 +0xd7
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:248 +0x2a2
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1030 +0x24c
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 41 (running) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1005 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1247 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1245 +0x527
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1162 +0x2ff
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223

Goroutine 159 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46
==================
--- FAIL: TestCycles (0.01s)
    testing.go:888: race detected during execution of test
==================
WARNING: DATA RACE
Write at 0x00c008825110 by goroutine 159:
  runtime.mapassign_faststr()
      /workdir/go/src/runtime/map_faststr.go:202 +0x0
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:992 +0x2c4
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Previous read at 0x00c008825110 by goroutine 41:
  runtime.mapiterinit()
      /workdir/go/src/runtime/map.go:797 +0x0
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:640 +0x1543
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb

Goroutine 159 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 41 (finished) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1005 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1247 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:954 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1245 +0x527
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1162 +0x2ff
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223
==================
[...]

/cc @matloob

@matloob
Copy link
Contributor

@matloob matloob commented Jan 23, 2020

I'm able to reproduce this on linux/amd64 with tools at golang/tools@774c71f and go built from 82a2f82. But it's incredibly rare...

It doesn't show up with -count=1000 but does with -count=10000.

@bcmills
Copy link
Member

@bcmills bcmills commented Feb 25, 2020

Still present: https://build.golang.org/log/ecaa511eb5aca0da535eb5c016e62c9ceb69c3f1

package nosuchpkg is not in GOROOT (/workdir/go/src/nosuchpkg)
package nosuchpkg is not in GOROOT (/workdir/go/src/nosuchpkg)
open /workdir/gopath/src/golang.org/x/tools/go/loader/missing.go: no such file or directory
open /workdir/gopath/src/golang.org/x/tools/go/loader/missing.go: no such file or directory
/workdir/gopath/src/golang.org/x/tools/go/loader/testdata/badpkgdecl.go:1:34: expected 'package', found 'EOF'
/workdir/gopath/src/golang.org/x/tools/go/loader/testdata/badpkgdecl.go:1:34: expected 'package', found 'EOF'
/go/src/b/x.go:1:21: could not import c (cannot find package "c" in any of:
	/go/src/c (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath'))
/go/src/b/x.go:1:21: could not import c (cannot find package "c" in any of:
	/go/src/c (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath'))
/go/src/b/x.go:1:21: could not import c (/go/src/c/x.go:1:8: expected 'IDENT', found 'EOF')
/go/src/c/x.go:1:20: expected operand, found 'EOF'
cannot find package "two/three" in any of:
	/go/src/two/three (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath')
cannot find package "http" in any of:
	/go/src/vendor/http (vendor tree)
	/go/src/http (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath')
/go/src/c/x.go:1:31: cannot convert false (untyped bool constant) to int
==================
WARNING: DATA RACE
Read at 0x00c000752e40 by goroutine 101:
  go/types.(*Package).Imports()
      /workdir/go/src/go/types/package.go:56 +0x2a1
  golang.org/x/tools/go/loader.markErrorFreePackages()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:686 +0x12f
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:669 +0x1d31
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb

Previous write at 0x00c000752e40 by goroutine 74:
  go/types.(*Checker).collectObjects()
      /workdir/go/src/go/types/resolver.go:264 +0x1c8a
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:255 +0xd7
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:248 +0x2a2
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1030 +0x24c
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 101 (running) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1043 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1300 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1298 +0x57f
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1210 +0x353
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223

Goroutine 74 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46
==================
==================
WARNING: DATA RACE
Read at 0x00c0011774c8 by goroutine 101:
  golang.org/x/tools/go/loader.markErrorFreePackages()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:686 +0x161
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:669 +0x1d31
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb

Previous write at 0x00c0011774c8 by goroutine 74:
  go/types.(*Checker).collectObjects()
      /workdir/go/src/go/types/resolver.go:264 +0x1c50
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:255 +0xd7
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:248 +0x2a2
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1030 +0x24c
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 101 (running) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1043 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1300 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1298 +0x57f
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1210 +0x353
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223

Goroutine 74 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46
==================
--- FAIL: TestCycles (0.01s)
    testing.go:906: race detected during execution of test
==================
WARNING: DATA RACE
Write at 0x00c002c30a50 by goroutine 74:
  runtime.mapassign_faststr()
      /workdir/go/src/runtime/map_faststr.go:202 +0x0
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:992 +0x2c4
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Previous read at 0x00c002c30a50 by goroutine 101:
  runtime.mapiterinit()
      /workdir/go/src/runtime/map.go:797 +0x0
  golang.org/x/tools/go/loader.(*Config).Load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:640 +0x1543
  golang.org/x/tools/go/loader_test.TestCycles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:744 +0xac2
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb

Goroutine 74 (running) created at:
  golang.org/x/tools/go/loader.(*importer).startLoad()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:969 +0x2bb
  golang.org/x/tools/go/loader.(*importer).importAll()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:884 +0x327
  golang.org/x/tools/go/loader.(*importer).addFiles()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:1014 +0x110
  golang.org/x/tools/go/loader.(*importer).load()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:989 +0x1f5
  golang.org/x/tools/go/loader.(*importer).startLoad.func1()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader.go:970 +0x46

Goroutine 101 (finished) created at:
  testing.(*T).Run()
      /workdir/go/src/testing/testing.go:1043 +0x660
  testing.runTests.func1()
      /workdir/go/src/testing/testing.go:1300 +0xa6
  testing.tRunner()
      /workdir/go/src/testing/testing.go:992 +0x1eb
  testing.runTests()
      /workdir/go/src/testing/testing.go:1298 +0x57f
  testing.(*M).Run()
      /workdir/go/src/testing/testing.go:1210 +0x353
  golang.org/x/tools/go/loader_test.TestMain()
      /workdir/gopath/src/golang.org/x/tools/go/loader/loader_test.go:32 +0x3d
  main.main()
      _testmain.go:90 +0x223
==================
--- FAIL: TestLoad2 (8.75s)
    testing.go:906: race detected during execution of test
--- FAIL: TestLoad1 (8.78s)
    testing.go:906: race detected during execution of test
FAIL
FAIL	golang.org/x/tools/go/loader	43.191s
@bcmills
Copy link
Member

@bcmills bcmills commented May 27, 2020

From #39133 (comment):

Given that the failure is detected in TestCycles, I suspect that it is related to this comment:

				// Cycle-forming import: we must not await its
				// completion since it would deadlock.
				//
				// We don't record the error in ii since
				// the error is really associated with the
				// cycle-forming edge, not the package itself.
				// (Also it would complicate the
				// invariants of importPath completion.)
@gopherbot
Copy link

@gopherbot gopherbot commented Feb 18, 2021

Change https://golang.org/cl/293836 mentions this issue: go/loader: fix race

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants