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: crash in analysisNode.typeCheck #64227

Open
husamettinarabaci opened this issue Nov 6, 2023 · 12 comments
Open

x/tools/gopls: crash in analysisNode.typeCheck #64227

husamettinarabaci opened this issue Nov 6, 2023 · 12 comments
Assignees
@adonovan
Labels
gopls/analysis Issues related to running analysis in gopls 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.16.0

Comments

@husamettinarabaci
Copy link

gopls version: v0.14.1 (go1.21.3)
gopls flags:
update flags: proxy
extension version: 0.39.1
go version: 1.21.3
environment: code-server linux
initialization error: undefined
issue timestamp: Mon, 06 Nov 2023 07:04:10 GMT
restart history:
Mon, 06 Nov 2023 07:03:37 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

Describe what you observed.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xc5b1f3]

goroutine 6230 [running]:
golang.org/x/tools/gopls/internal/lsp/cache.(*analysisNode).typeCheck(0xc00330db80, {0xc005d43180, 0xa, 0xa})
	  analysis.go:1042  0xbd3
golang.org/x/tools/gopls/internal/lsp/cache.(*analysisNode).run(0xc00330db80, {0x122ae08%3F, 0xc0041a9f20})
	  analysis.go:794  0x206
golang.org/x/tools/gopls/internal/lsp/cache.(*analysisNode).runCached(0xc00330db80, {0x122ae08%3F, 0xc0041a9f20})
	  analysis.go:656  0x12d
golang.org/x/tools/gopls/internal/lsp/cache.(*snapshot).Analyze.func6.1()
	  analysis.go:383  0xc9
golang.org/x/sync/errgroup.(*Group).Go.func1()
	  errgroup.go:75  0x56
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 6229
	  errgroup.go:72  0x96
[Error - 7:03:52 AM]

OPTIONAL: If you would like to share more information, you can attach your complete gopls logs.

NOTE: THESE MAY CONTAIN SENSITIVE INFORMATION ABOUT YOUR CODEBASE.
DO NOT SHARE LOGS IF YOU ARE WORKING IN A PRIVATE REPOSITORY.

<OPTIONAL: ATTACH LOGS HERE>
@findleyr
Copy link
Contributor

findleyr commented Nov 6, 2023

Hi, thanks very much for the report.

Is this reproducible on your end, or did restarting the language server make it go away?

@suzmue suzmue added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 10, 2023
@husamettinarabaci
Copy link
Author

Hi,
No, It wasn't. It was solved after restarting the language server.

@findleyr
Copy link
Contributor

Hi, reopening since all crashes warrant investigation. If it didn't reoccur upon restart, that may just suggest that there is an internal race that is rare.

@findleyr findleyr reopened this Nov 17, 2023
@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: crash in analysisNode.typeCheck Nov 17, 2023
@findleyr findleyr transferred this issue from golang/vscode-go Nov 17, 2023
@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 Nov 17, 2023
@gopherbot gopherbot added this to the Unreleased milestone Nov 17, 2023
@findleyr findleyr removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 17, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.15.0 Nov 17, 2023
@findleyr
Copy link
Contributor

@adonovan the crash is on this line:

https://cs.opensource.google/go/x/tools/+/refs/tags/gopls/v0.14.1:gopls/internal/lsp/cache/analysis.go;l=1042;drc=b9b97d982b0a16d0bc8d95fb1dc654120b0d4940

In makeNode, we verify that the metadata is non-nil (hooray for such assertions!):
https://cs.opensource.google/go/x/tools/+/refs/tags/gopls/v0.14.1:gopls/internal/lsp/cache/analysis.go;l=236;drc=b9b97d982b0a16d0bc8d95fb1dc654120b0d4940

Therefore, it seems that it must be the case that the analysisNode.summary is nil... but I'll admit I stared at the code for 10m and did not see a bug: a node should not run until its successors have completed, all deps should be present in the succs map, there can be at most one per ID, and once a successor has completed its summary must be present. Perhaps I'm missing something, but there didn't appear to be an obvious bug.

@husamettinarabaci
Copy link
Author

How can I help with your investigation?

@adonovan adonovan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls/analysis Issues related to running analysis in gopls labels Nov 21, 2023
@adonovan adonovan self-assigned this Dec 1, 2023
@JamyDev
Copy link

JamyDev commented Dec 4, 2023

@husamettinarabaci do you happen to use Bazel/rules_go in the project that was having this issue?

@husamettinarabaci
Copy link
Author

@JamyDev https://bazel.build/ did you mean this?

@JamyDev
Copy link

JamyDev commented Dec 5, 2023

Yeah, I mentioned this because we had the exact same error with the bazel toolchain active. But if you're not using it, it's unlikely to be the root cause.

@dt
Copy link
Contributor

dt commented Dec 20, 2023 •
edited

@JamyDev on the bazel/rules_go side, potentially related, I started hitting this crash after upgrading rules_go to 0.43 today, but only immediately after I open a test file that uses an xtest package.

EDIT: nevermind, I see now that that was addressed in bazelbuild/rules_go#3777

@hyangah hyangah mentioned this issue Jan 12, 2024
Closed
@adonovan
Copy link
Member

The possible dup at golang/vscode-go#3126 raises the question of whether this is a package cycle that somehow slipped through the cycle breaking in detectImportCycles.

@findleyr
Copy link
Contributor

So, one observation: the repro in golang/vscode-go#3126 had a self-cycle, and detectImportCycles/breakImportCycles appears not to work for 1-cycles. Furthermore, an.allDeps has an explicit self-reference (it is the reflexive transitive closure).

If (1) go/packages could be tricked to return a 1-cycle, or (2) we somehow got a self reference in the export data manifest, it would result in this crash.

But of course I only need such a long explanation because I don't have a repro, despite my attempts.

@gopherbot
Copy link

Change https://go.dev/cl/560463 mentions this issue: gopls/internal/cache/metadata: assert graph is acyclic

gopherbot pushed a commit to golang/tools that referenced this issue Feb 2, 2024
@adonovan
gopls/internal/cache/metadata: assert graph is acyclic
efce0f5 
This change causes the metadata graph to assert that it is
acyclic before applying the updates. This should be an
invariant, but the attached issues make us skeptical.

Updates golang/go#64227
Updates golang/vscode-go#3126

Change-Id: I40b4fd06fcf2c64594b34b8c300f20ca0676d0fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/560463
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@adonovan adonovan modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
gopls/analysis Issues related to running analysis in gopls 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.16.0
Development

No branches or pull requests
7 participants
@dt @JamyDev @adonovan @husamettinarabaci @suzmue @gopherbot @findleyr