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/internal/lsp/cache: fails to load packages with missing imports concurrently #35951

Closed
heschi opened this issue Dec 4, 2019 · 1 comment
Closed

x/tools/internal/lsp/cache: fails to load packages with missing imports concurrently #35951

heschi opened this issue Dec 4, 2019 · 1 comment
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.4.0

Comments

@heschi
Copy link
Contributor

heschi commented Dec 4, 2019

If (*snapshot).PackageHandles is called concurrently, the following race is possible:

g1: PackageHandles starts, calls getMetadataForURI, and enters load, which calls packages.Load.
g2: does the same.
g1: packages.Load finishes. load re-reads metadata inside of updateMetadata, then updates it. It returns all the way up to PackageHandles and is fine.
g2: packages.Load finishes. load re-reads metadata inside of updateMetadata. It observes the changes from g1, and in validateMetadata concludes that it does not need to recheck the package. It returns nil to PackageHandles, indicating that it should use the previous metadata. Unfortunately, that previous metadata was read before g2 finished, and is empty. PackageHandles then returns an error.

Obviously the bug should be fixed. It might also make sense to deduplicate load calls at some level, or limit the number of outstanding packages.Load calls somehow.

cc @stamblerre @ianthehat
@gopherbot gopherbot added this to the Unreleased milestone Dec 4, 2019
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 4, 2019
@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 4, 2019
@gopherbot
Copy link

Change https://golang.org/cl/209737 mentions this issue: internal/lsp/cache: fix load race, refactor

@heschi heschi mentioned this issue Dec 6, 2019
Closed
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020
@golang golang locked and limited conversation to collaborators Jul 22, 2021
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. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.4.0
Development

No branches or pull requests
3 participants
@stamblerre @gopherbot @heschi