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

Jump to bottom

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
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

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
@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 6, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jan 6, 2020
@matloob
Copy link
Contributor

matloob commented Jan 23, 2020

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

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

@bcmills
Copy link
Contributor

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

@stamblerre stamblerre mentioned this issue May 27, 2020
Closed
@bcmills
Copy link
Contributor

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.)

@bcmills bcmills mentioned this issue May 28, 2020
Closed
@mvdan
Copy link
Member

mvdan commented Jun 18, 2020

Again on https://go-review.googlesource.com/c/tools/+/238478: https://storage.googleapis.com/go-build-log/377c1536/linux-amd64-race_596f3da2.log

@gopherbot
Copy link

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

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 23, 2021
@golang golang locked and limited conversation to collaborators Feb 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@dmitshur @mvdan @bcmills @gopherbot @matloob