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/gopls: SignatureHelp crash (reported by telemetry) #66090

Closed
adonovan opened this issue Mar 4, 2024 · 6 comments
Closed

x/tools/gopls: SignatureHelp crash (reported by telemetry) #66090

adonovan opened this issue Mar 4, 2024 · 6 comments
Assignees
@findleyr
Labels
gopls/telemetry-wins 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. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.15.2

Comments

@adonovan
Copy link
Member

adonovan commented Mar 4, 2024

This stack GpSf_A was reported by telemetry:

crash/crash
runtime.gopanic:+69
runtime.panicmem:=261
runtime.sigpanic:+19
golang.org/x/tools/gopls/internal/server.(*server).SignatureHelp:+7
golang.org/x/tools/gopls/internal/protocol.serverDispatch:+544
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
runtime.goexit:+0

golang.org/x/tools/gopls@v0.15.0-pre.3 devel linux/amd64 other (2)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.
@adonovan adonovan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. gopls/telemetry-wins labels Mar 4, 2024
@gopherbot gopherbot added this to the Unreleased milestone Mar 4, 2024
@adonovan
Copy link
Member Author

adonovan commented Mar 4, 2024 •
edited

This is a strange one. The indicated line of SignatureHelp is infallible, as are its immediate neighbors. I wonder whether there is a bug in the processing of stack frames. (The neighboring frames +/-2 all look sound though.)

@findleyr
Copy link
Contributor

Here's another example of this:
https://storage.googleapis.com/prod-telemetry-merged/2024-03-09.json

@findleyr
Copy link
Contributor

Aha, the panic is because release is deferred before the error is checked. Let's fix for gopls@v0.15.2.

@findleyr findleyr self-assigned this Mar 11, 2024
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.15.2 Mar 11, 2024
@gopherbot
Copy link

Change https://go.dev/cl/570675 mentions this issue: gopls/internal/server: fix crash in SignatureHelp

@gopherbot
Copy link

Change https://go.dev/cl/570596 mentions this issue: [gopls-release-branch.0.15] gopls/internal/server: fix crash in SignatureHelp

@adonovan
Copy link
Member Author

The line number mystery has a simple explanation: a panic caused by executing a deferred function that is nil is reported at the return statement: https://go.dev/play/p/TRTwguxqivI?v=gotip

gopherbot pushed a commit to golang/tools that referenced this issue Mar 11, 2024
@findleyr @gopherbot
[gopls-release-branch.0.15] gopls/internal/server: fix crash in Signa…
ba252e8 
…tureHelp

Fix a crash when Snapshot.fileOf fails, due to e.g. context
cancellation. The defer of release should only occur after the error is
checked. Unfortunately, this is very hard to test since it likely occurs
only due to races.

This is our first bug found completely via automated crash reporting.

Fixes golang/go#66090

Change-Id: I7c22b5c2d1a0115718cd4bf2b84c31cc38a891ec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/570675
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit f89da53)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/570596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@findleyr findleyr

Labels
gopls/telemetry-wins 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. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.15.2
Development

No branches or pull requests
3 participants
@adonovan @gopherbot @findleyr