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.

Sign up for GitHub

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/cmd/gopls: data race during package load #30107

Closed
pwaller opened this issue Feb 6, 2019 · 2 comments
Closed

x/tools/cmd/gopls: data race during package load #30107

pwaller opened this issue Feb 6, 2019 · 2 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@pwaller
Copy link
Contributor

pwaller commented Feb 6, 2019

What version of Go are you using (go version)?

$ go version
go version devel +4b3f04c63b Thu Jan 10 18:15:48 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes, I am testing on master (40960b6) at time of writing.

What did you do?

Compile gopls with -race. Use cmd/gopls/forward. Use vscode-go. I am working out of my $GOPATH using modules, and have GO111MODULE=on.

The race manifests reliably and almost immediately after loading the editor.

What did you expect to see?

Things working as normal.

What did you see instead?

The race detector reports warnings.

Additional notes

I have verified that the mutex held in GetPackage() is the same one every time by adding the following log statement: log.Printf("operating with view mu: %p", &f.view.mu). It always reports the same value.

It's interesting because it looks like the lsp code is doing the reasonable thing to prevent concurrent writes, but something is going on with the handling of float constants inside the type checker.

Data race warnings:

==================
WARNING: DATA RACE
Write at 0x00c00a331ce8 by goroutine 12:
  runtime.slicecopy()
      /home/pwaller/.local/src/go/src/runtime/slice.go:197 +0x0
  math/big.(*Rat).Float64()
      /home/pwaller/.local/src/go/src/math/big/nat.go:94 +0x1e3
  go/constant.Float64Val()
      /home/pwaller/.local/src/go/src/go/constant/value.go:522 +0x33d
  go/types.roundFloat64()
      /home/pwaller/.local/src/go/src/go/types/expr.go:175 +0x46
  go/types.representableConst()
      /home/pwaller/.local/src/go/src/go/types/expr.go:279 +0x838
  go/types.(*Checker).conversion()
      /home/pwaller/.local/src/go/src/go/types/conversions.go:21 +0x947
  go/types.(*Checker).call()
      /home/pwaller/.local/src/go/src/go/types/call.go:34 +0x325
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1469 +0x2ca8
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1489 +0x28f8
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).initVars.func1()
      /home/pwaller/.local/src/go/src/go/types/assignments.go:209 +0xa0
  go/types.(*Checker).initVars()
      /home/pwaller/.local/src/go/src/go/types/assignments.go:246 +0x710
  go/types.(*Checker).stmt()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:447 +0x5571
  go/types.(*Checker).stmtList()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:120 +0xf0
  go/types.(*Checker).funcBody()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:42 +0x370
  go/types.(*Checker).funcDecl.func1()
      /home/pwaller/.local/src/go/src/go/types/decl.go:561 +0xf1
  go/types.(*Checker).processDelayed()
      /home/pwaller/.local/src/go/src/go/types/resolver.go:615 +0x59
  go/types.(*Checker).checkFiles()
      /home/pwaller/.local/src/go/src/go/types/check.go:256 +0xc9
  golang.org/x/tools/go/packages.(*loader).loadPackage()
      /home/pwaller/.local/src/go/src/go/types/check.go:245 +0xcaf
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:573 +0x258
  sync.(*Once).Do()
      /home/pwaller/.local/src/go/src/sync/once.go:44 +0xde
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:560 +0x74
  golang.org/x/tools/go/packages.(*loader).refine.func2()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:541 +0x42

Previous write at 0x00c00a331ce8 by goroutine 542:
  runtime.slicecopy()
      /home/pwaller/.local/src/go/src/runtime/slice.go:197 +0x0
  math/big.(*Rat).Float64()
      /home/pwaller/.local/src/go/src/math/big/nat.go:94 +0x1e3
  go/constant.Float64Val()
      /home/pwaller/.local/src/go/src/go/constant/value.go:522 +0x33d
  go/types.roundFloat64()
      /home/pwaller/.local/src/go/src/go/types/expr.go:175 +0x46
  go/types.representableConst()
      /home/pwaller/.local/src/go/src/go/types/expr.go:279 +0x838
  go/types.(*Checker).conversion()
      /home/pwaller/.local/src/go/src/go/types/conversions.go:21 +0x947
  go/types.(*Checker).call()
      /home/pwaller/.local/src/go/src/go/types/call.go:34 +0x325
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1469 +0x2ca8
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1112 +0x163a
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).expr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1593 +0x56
  go/types.(*Checker).exprInternal()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1489 +0x28f8
  go/types.(*Checker).rawExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:982 +0x91
  go/types.(*Checker).multiExpr()
      /home/pwaller/.local/src/go/src/go/types/expr.go:1599 +0x68
  go/types.(*Checker).initVars.func1()
      /home/pwaller/.local/src/go/src/go/types/assignments.go:209 +0xa0
  go/types.(*Checker).initVars()
      /home/pwaller/.local/src/go/src/go/types/assignments.go:246 +0x710
  go/types.(*Checker).stmt()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:447 +0x5571
  go/types.(*Checker).stmtList()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:120 +0xf0
  go/types.(*Checker).funcBody()
      /home/pwaller/.local/src/go/src/go/types/stmt.go:42 +0x370
  go/types.(*Checker).funcDecl.func1()
      /home/pwaller/.local/src/go/src/go/types/decl.go:561 +0xf1
  go/types.(*Checker).processDelayed()
      /home/pwaller/.local/src/go/src/go/types/resolver.go:615 +0x59
  go/types.(*Checker).checkFiles()
      /home/pwaller/.local/src/go/src/go/types/check.go:256 +0xc9
  golang.org/x/tools/go/packages.(*loader).loadPackage()
      /home/pwaller/.local/src/go/src/go/types/check.go:245 +0xcaf
  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:573 +0x258
  sync.(*Once).Do()
      /home/pwaller/.local/src/go/src/sync/once.go:44 +0xde
  golang.org/x/tools/go/packages.(*loader).loadRecursive()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:560 +0x74
  golang.org/x/tools/go/packages.(*loader).refine.func2()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:541 +0x42

Goroutine 12 (running) created at:
  golang.org/x/tools/go/packages.(*loader).refine()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:540 +0xf3c
  golang.org/x/tools/go/packages.Load()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:177 +0x1b8
  golang.org/x/tools/internal/lsp/cache.(*View).parse()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/view.go:101 +0x191
  golang.org/x/tools/internal/lsp/cache.(*File).GetPackage()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/file.go:73 +0x1b6
  golang.org/x/tools/internal/lsp/source.Diagnostics()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/source/diagnostics.go:30 +0x144
  golang.org/x/tools/internal/lsp.(*server).cacheAndDiagnose.func1()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/diagnostics.go:24 +0xb3

Goroutine 542 (running) created at:
  golang.org/x/tools/go/packages.(*loader).refine()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:540 +0xf3c
  golang.org/x/tools/go/packages.Load()
      /home/pwaller/.local/src/golang.org/x/tools/go/packages/packages.go:177 +0x1b8
  golang.org/x/tools/internal/lsp/cache.(*View).parse()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/view.go:101 +0x191
  golang.org/x/tools/internal/lsp/cache.(*File).GetPackage()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/cache/file.go:73 +0x1b6
  golang.org/x/tools/internal/lsp/source.Diagnostics()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/source/diagnostics.go:30 +0x144
  golang.org/x/tools/internal/lsp.(*server).cacheAndDiagnose.func1()
      /home/pwaller/.local/src/golang.org/x/tools/internal/lsp/diagnostics.go:24 +0xb3
==================

/cc @stamblerre
@gopherbot gopherbot added this to the Unreleased milestone Feb 6, 2019
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 6, 2019
@andybons andybons changed the title x/tools/cmd/gopls: Data race during package load x/tools/cmd/gopls: data race during package load Feb 6, 2019
@stamblerre
Copy link
Contributor

/cc @matloob

@stamblerre stamblerre added gopls Issues related to the Go language server, gopls. and removed gopls Issues related to the Go language server, gopls. labels Mar 12, 2019
@stamblerre
Copy link
Contributor

I believe this bug was filed about a much older version of gopls that still used go/packages.Load to type-check packages. gopls has changed a lot since then and now type-checks packages from source. If this issue comes up again, please create a new bug.

@bcmills bcmills mentioned this issue Jun 28, 2019
Closed
@golang golang locked and limited conversation to collaborators May 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@andybons @pwaller @stamblerre @gopherbot