x/tools/gopls: crash in analysisNode.typeCheck

[husamettinarabaci]

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]

Hi, thanks very much for the report.

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

[husamettinarabaci]

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

[findleyr]

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]

@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]

How can I help with your investigation?

[JamyDev]

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

[husamettinarabaci]

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

[JamyDev]

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]

@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 bazel-contrib/rules_go#3777

[adonovan]

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]

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]

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

[gopherbot]

Change https://go.dev/cl/622038 mentions this issue: gopls/internal/cache: share type checking with analysis