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/gopls: stale diagnostic, possibly from an orphaned workspace package #43347

Open
findleyr opened this issue Dec 23, 2020 · 1 comment
Open

Comments

@findleyr
Copy link
Contributor

@findleyr findleyr commented Dec 23, 2020

I encountered a strange and concerning bug in gopls yesterday (using gopls/v0.6.1), which presented as follows:

While working on the regtests, I typed e.Sandbox.RunGoCommand (a function), at which point I got a go/types diagnostic for the unused ExprStmt. I then kept typing, but the "is unused" diagnostic stuck around. Subsequently editing and saving the file did not eliminate or move the diagnostic -- it was republished for each snapshot. My first thought is that this is a bug in diagnostic caching, but I don't yet see how that could be possible and the following evidence suggests otherwise: I have a diagnostic delay configured and the initial publish after editing (which only diagnoses the PackagesForFile) cleared the erroneous diagnosic, then it came back on the subsequent delayed diagnostics pass, which diagnoses all workspace packages.

We (@stamblerre and I) therefore suspected that I somehow had an orphaned workspace package which is no longer associated with the file I was editing. If this were possible we would not invalidate this typechecked package upon edits to the file, its stale diagnostics would persist across snapshots, and it would not be part of the PackagesForFile package set and therefore wouldn't be present in the initial diagnostics pass. In other words, the symptoms would be exactly what I observed. On the other hand I don't yet see how a package could be orphaned in such a way, and nothing I did while editing the file would have triggered abnormal code paths (in my RPC logs there are no nearby packages.Load calls that would have affected package associations).

So after staring at this for a bit, I'm currently at a loss. While the bug persisted in my gopls session, I unfortunately did not have the debug server enabled and was unsurprisingly not able to reproduce in a new session. Eventually, after an ~hour in the broken session, the stale diagnostic just went away.

This is likely a rare enough bug that it is relatively low impact, but its nature demonstrates that some invariant of the gopls server is being broken. I therefore think we should proceed by instrumenting gopls to assert loudly on the invariants we expect: logging errors in production builds and crashing in development builds.

CC @stamblerre @heschik @pjweinbgo

@findleyr findleyr added this to the gopls/v0.6.2 milestone Dec 23, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default via automation Dec 23, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 30, 2020

Change https://golang.org/cl/280698 mentions this issue: internal/lsp/cache: add a check for snapshot invariants

gopherbot pushed a commit to golang/tools that referenced this issue Dec 30, 2020
Check that some invariants are met when cloning the snapshot, in an
attempt to catch golang/go#43347

For golang/go#43347

Change-Id: I7404509027a1b0b0085133cba4d21d1006a52a57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280698
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre removed this from the gopls/v0.6.2 milestone Dec 31, 2020
@stamblerre stamblerre added this to the gopls/vscode-go milestone Dec 31, 2020
@stamblerre stamblerre moved this from Needs Triage to Non-critical in vscode-go: gopls by default Jan 6, 2021
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Jan 6, 2021
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jan 6, 2021
@stamblerre stamblerre removed this from Non-critical in vscode-go: gopls by default Jan 6, 2021
@stamblerre stamblerre added this to To Do in gopls on-deck Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants