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: crash importing test variant of tools file #66109

Closed
painhardcore opened this issue Mar 1, 2024 · 17 comments
Closed

x/tools/gopls: crash importing test variant of tools file #66109

painhardcore opened this issue Mar 1, 2024 · 17 comments
Assignees
Labels
gopls/metadata Issues related to metadata loading in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@painhardcore
Copy link

gopls version: v0.15.1/go1.22.0
gopls flags:
update flags: proxy
extension version: 0.41.1
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Fri, 01 Mar 2024 00:31:32 GMT
restart history:
Fri, 01 Mar 2024 00:30:44 GMT: activation (enabled: true)

Crashing gopls

panic: nil metadata for "github.com/mailru/easyjson/easyjson [command-line-arguments.test]"

goroutine 7085 [running]:
golang.org/x/tools/gopls/internal/cache.(*Snapshot).getPackageHandles.func1(0x1400d4f3950, {0x14009c2b040, 0x41})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/check.go:872 +0x53c
golang.org/x/tools/gopls/internal/cache.(*Snapshot).getPackageHandles.func1(0x0, {0x14009c54500, 0x7a})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/check.go:889 +0x468
golang.org/x/tools/gopls/internal/cache.(*Snapshot).getPackageHandles(0x140021d59e0, {0x10104c648, 0x14008365590}, {0x1400b2c9b08, 0x1, 0x0?})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/check.go:899 +0x17c
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackage(0x140021d59e0, {0x10104c648, 0x14008365320}, {0x1400b2c9b08, 0x1, 0x1}, 0x1400a266c80, 0x1400cb41ca0)
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/check.go:343 +0x134
golang.org/x/tools/gopls/internal/cache.(*Snapshot).PackageDiagnostics(0x140021d59e0, {0x10104c680?, 0x1400582e6e0?}, {0x1400b2c9b08, 0x1, 0x1})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/snapshot.go:686 +0x19c
golang.org/x/tools/gopls/internal/cache.(*Snapshot).orphanedFileDiagnostics(0x140021d59e0, {0x10104c680, 0x1400582e6e0}, {0x1400a266c00, 0x3, 0x10104aaf8?})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/snapshot.go:1452 +0x1c8
golang.org/x/tools/gopls/internal/cache.(*Session).OrphanedFileDiagnostics(0x1400048e150, {0x10104c680, 0x1400582e6e0})
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/cache/session.go:1165 +0x474
golang.org/x/tools/gopls/internal/server.(*server).diagnoseChangedViews(0x14000190000, {0x10104c680, 0x1400582e6e0}, 0x3, 0x140086b0b40, 0x0)
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/server/diagnostics.go:157 +0x4f0
golang.org/x/tools/gopls/internal/server.(*server).didModifyFiles.func2()
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/server/text_synchronization.go:269 +0x3c
created by golang.org/x/tools/gopls/internal/server.(*server).didModifyFiles in goroutine 108
	/Users/painhardcore/go/pkg/mod/golang.org/x/tools/gopls@v0.15.1/internal/server/text_synchronization.go:268 +0x468
[Error - 01:31:32] 

gopls stats -anon { "DirStats": { "Files": 8067, "TestdataFiles": 52, "GoFiles": 6683, "ModFiles": 4, "Dirs": 1348 }, "GOARCH": "arm64", "GOOS": "darwin", "GOPACKAGESDRIVER": "", "GOPLSCACHE": "", "GoVersion": "go1.22.0", "GoplsVersion": "v0.15.1", "InitialWorkspaceLoadDuration": "1.65849025s", "MemStats": { "HeapAlloc": 108681552, "HeapInUse": 144924672, "TotalAlloc": 294147416 }, "WorkspaceStats": { "Files": { "Total": 6971, "Largest": 7361676, "Errs": 0 }, "Views": [ { "GoCommandVersion": "go1.22.0", "AllPackages": { "Packages": 1426, "LargestPackage": 214, "CompiledGoFiles": 8363, "Modules": 177 }, "WorkspacePackages": { "Packages": 593, "LargestPackage": 214, "CompiledGoFiles": 4074, "Modules": 1 }, "Diagnostics": 0 } ] } }
@findleyr
Copy link
Contributor

findleyr commented Mar 1, 2024

Thanks for the report. This looks like a dupe of golang/vscode-go#3207, and it's very interesting that the package ID in question is exactly the same. Are you by any chance a colleague of @amirrmonfared? I ask because if this isn't coming from the exact same code base, it's possible that there's something about the package layout of easyjson that exacerbates this bug.

@findleyr
Copy link
Contributor

findleyr commented Mar 4, 2024

@painhardcore I should also ask: does this crash occur in an open source repository, from which we could try to reproduce?

@painhardcore
Copy link
Author

@findleyr Yep, his my colleague and has similar issue. Not sure about open source repo that we can match(we have vendored packages and easyjson)...

1.15.0 and 1.15.1 are affected and have same panic trace, but 1.14.2 works fine

But so far I found a small trick to work

  1. Close VScode
  2. Downgrade to stable that works go install golang.org/x/tools/gopls@v0.14.2
  3. Open VScode

VScode will update the gopls anyway, but the current gopls instance wont be effected. So if I need other brach in a new window(I use git worktree) - I just downgrade it before opening new VScode.

@findleyr
Copy link
Contributor

findleyr commented Mar 4, 2024

But so far I found a small trick to work

To be clear, downgrading is not a long-term solution. We need to fix this bug.

I believe this problem is related to the use of a tools.go pattern, as is described here:
https://marcofranssen.nl/manage-go-tools-via-go-modules

"github.com/mailru/easyjson/easyjson" is a command, not an importable package (i.e. it is package main), so the only reason to have it in an import statement would be to intentionally introduce a tool dependency. However, I have tried many different ways of importing it, and can't produce a crash.

Can you search for this import in your codebase, and share details about the file containing the import? If possible, I would love to see the full header of the file with the import (the package name and imports, as well as any //go:build comments). I'd also like to know whether the file is a _test.go file. Does the crash occur as soon as you open your repository, or only when the importing file is open?

@painhardcore
Copy link
Author

Yep, we have a file, that we keep to have everything in vendor.

//go:build tools
// +build tools

// This file tracks toolings that aren't directly used by repo
// but they are required to build the project.
// Refer to https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
package back

import (
	_ "github.com/mailru/easyjson/easyjson"
	_ "github.com/quasilyte/go-ruleguard/dsl" // For .github/gorules/rules.go.
)

I don't understand how its connected with _test.go files, but apparently something triggers background scan or smth:

I can go and edit .go files and everything is OK. Until I go to _test.go file and do "go to definition" it's crashing.

But if I remove that file with tools - it works OK, and crashes only when I bring it back.

@painhardcore
Copy link
Author

@painhardcore
Copy link
Author

@findleyr and I found the issue, its because the file with these tools is called tools_test.go 🤣
And apparently if we name it tools.go - everything work without panics. We need to doublecheck with @amirrmonfared just to be sure.
I don't know it's a bug but it worked before.

@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: crash importing test variant of tools file Mar 4, 2024
@findleyr
Copy link
Contributor

findleyr commented Mar 4, 2024

Thanks. For some reason I still can't reproduce, but this is enough information for me to put together a fix. Transferring to the gopls issue tracker.

@findleyr findleyr transferred this issue from golang/vscode-go Mar 4, 2024
@findleyr findleyr added this to the gopls/v0.15.2 milestone Mar 4, 2024
@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 Mar 4, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569035 mentions this issue: gopls/internal/cache: prune broken edges to command-line-arguments pkgs

@painhardcore
Copy link
Author

painhardcore commented Mar 4, 2024

@findleyr reproducable repo https://github.com/painhardcore/goplscrash . Just go to tools/tools_test.go in VScode and gopls will crash.

@findleyr
Copy link
Contributor

findleyr commented Mar 4, 2024

Indeed it does! Apparently the file must be in a nested directory. Thank you for the reproducer!

@painhardcore
Copy link
Author

Yep, in our case file tools_test.go was in root folder, so every test file in a big repo was like a trigger 😄

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569437 mentions this issue: [gopls-release-branch.0.15] gopls/internal/cache: prune broken edges to command-line-arguments pkgs

@findleyr
Copy link
Contributor

findleyr commented Mar 6, 2024

@painhardcore and @amirrmonfared, huge thanks for your conscientious help resolving this bug. The CL above should fix this crash. However, we have a couple other issues to sort out before we cut gopls@v0.15.2 with this fix.

@painhardcore
Copy link
Author

@painhardcore and @amirrmonfared, huge thanks for your conscientious help resolving this bug. The CL above should fix this crash. However, we have a couple other issues to sort out before we cut gopls@v0.15.2 with this fix.

Thanks.
I fixed the issue in our repo, so we can work on 0.15.1

gopherbot pushed a commit to golang/tools that referenced this issue Mar 6, 2024
…to command-line-arguments pkgs

Fix two bugs discovered during the investigation of golang/go#66109,
which revealed the strange and broken intermediate test variant form
"path/to/command/package [command-line-arguments.test]", referenced from
the equally broken
"command-line-arguments [command-line-arguments.test]". This latter
package was *also* detected as an ITV, which is why we never tried to
type check it in gopls@v0.14.2.

- Snapshot.orphanedFileDiagnostics was not pruning intermediate test
  variants, causing it to be the one place where we were now type
  checking ITVs.
- Fix the latent bug that caused gopls to record a dangling edge between
  the two ITVs.

There is a third bug in go/packages, filed as golang/go#66126.

Fixes golang/go#66109

Change-Id: Ie5795b6d5a4831bf2f73217c8eb22c6ba18e59cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/569035
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
(cherry picked from commit caf5940)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/569437
Auto-Submit: Robert Findley <rfindley@google.com>
@amirrmonfared
Copy link

Thanks a lot @findleyr for your support and thanks to @painhardcore for solving the issue in our repo ❤️

@findleyr
Copy link
Contributor

findleyr commented Mar 8, 2024

This fix is included in our prerelease:

go install golang.org/x/tools/gopls@v0.15.2-pre.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/metadata Issues related to metadata loading in gopls 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
Development

No branches or pull requests

5 participants