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

go/types: data race in computeInterfaceTypeSet #54667

Closed
adonovan opened this issue Aug 24, 2022 · 4 comments
Closed

go/types: data race in computeInterfaceTypeSet #54667

adonovan opened this issue Aug 24, 2022 · 4 comments
Assignees
@adonovan
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.

Comments

@adonovan
Copy link
Member

See https://farmer.golang.org/try?commit=79d5f0e3, linux-amd64-race
Change-ID: I0576d03fcfffe0c8df157cf6c6520c9d402f8803
Commit: 79d5f0e35886839bd2f67d2485de47566d51ca26
Log: https://storage.googleapis.com/go-build-log/e4bed415/linux-amd64-race_db9e5b45.log

The first example is typical:

WARNING: DATA RACE
Read at 0x00c0002ac958 by goroutine 147:
  go/types.computeInterfaceTypeSet()
      /workdir/go/src/go/types/typeset.go:154 +0x6f
  go/types.(*Checker).validVarType.func1()
      /workdir/go/src/go/types/typexpr.go:163 +0xc4
  go/types.(*Checker).processDelayed()
      /workdir/go/src/go/types/check.go:390 +0x61
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:335 +0x18d
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:307 +0x142a
  golang.org/x/tools/go/packages.(*loader).loadPackage()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:1001 +0x12f9
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:838 +0x324
  sync.(*Once).doSlow()
      /workdir/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /workdir/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:826 +0x66
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:833 +0x44
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.2()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:835 +0x47

Previous write at 0x00c0002ac958 by goroutine 209:
  go/types.computeInterfaceTypeSet()
      /workdir/go/src/go/types/typeset.go:193 +0x1b1
  go/types.(*Checker).validVarType.func1()
      /workdir/go/src/go/types/typexpr.go:163 +0xc4
  go/types.(*Checker).processDelayed()
      /workdir/go/src/go/types/check.go:390 +0x61
  go/types.(*Checker).checkFiles()
      /workdir/go/src/go/types/check.go:335 +0x18d
  go/types.(*Checker).Files()
      /workdir/go/src/go/types/check.go:307 +0x142a
  golang.org/x/tools/go/packages.(*loader).loadPackage()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:1001 +0x12f9
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:838 +0x324
  sync.(*Once).doSlow()
      /workdir/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /workdir/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:826 +0x66
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:833 +0x44
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.2()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:835 +0x47

Goroutine 147 (running) created at:
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:832 +0xf0
  sync.(*Once).doSlow()
      /workdir/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /workdir/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:826 +0x66
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:833 +0x44
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.2()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:835 +0x47

Goroutine 209 (finished) created at:
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:832 +0xf0
  sync.(*Once).doSlow()
      /workdir/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /workdir/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:826 +0x66
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:833 +0x44
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.2()
      /workdir/gopath/src/golang.org/x/tools/go/packages/packages.go:835 +0x47
==================
==================
@adonovan adonovan self-assigned this Aug 24, 2022
@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 24, 2022
@adonovan
Copy link
Member Author

My guess is that there is some call to newInterface (e.g. here) that isn't finalized by setting of tset, causing the incomplete interface to be exposed. When it is accessed concurrently during type-checking of two dependent packages, both race to fill in the type set.

@griesemer

@mdempsky
Copy link
Member

This looks like a duplicate of #54653?

@mdempsky
Copy link
Member

I think you just need to rebase past go.dev/cl/425362.

@adonovan
Copy link
Member Author

Thanks Matt, this did appear to be the cause. I had initially suspected a second root cause because I didn't realize this test uses export data, but then I remembered that it doesn't use facts, allowing it to optimize by reading export data instead of syntax. Adding a nonempty FactTypes list to the checker_test.go caused the race to go away, and the cause and effect were confirmed in an A/B/A/B test.

@golang golang locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@adonovan adonovan

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mdempsky @adonovan @gopherbot @seankhliao