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/internal/lsp: crash on acme-lsp startup #34212

Closed
mariusae opened this issue Sep 10, 2019 · 4 comments
Closed

x/tools/internal/lsp: crash on acme-lsp startup #34212

mariusae opened this issue Sep 10, 2019 · 4 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls.
Milestone

Comments

@mariusae
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/marius/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/marius/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/marius/.gimme/versions/go1.12.1.darwin.amd64"
GOTMPDIR=""
GOTOOLDIR="/Users/marius/.gimme/versions/go1.12.1.darwin.amd64/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/6k/zldf6tdx42q2tdqd841n74gw0000gn/T/go-build782091440=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

When using acme-lsp with gopls, gopls crashes with a panic on startup.

What did you expect to see?

Not to panic.

What did you see instead?

A panic:

$ gopls -debug localhost:8081 -listen localhost:8080 serve  panic: runtime error: index out of range  goroutine 44 [running]: golang.org/x/tools/internal/lsp/cache.(*session).bestView(0xc00012f900, 0xc000026420, 0x57, 0x57, 0x1c5f300) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/cache/session.go:172 +0x198 golang.org/x/tools/internal/lsp/cache.(*session).ViewOf(0xc00012f900, 0xc000026420, 0x57, 0x0, 0x0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/cache/session.go:140 +0xdd golang.org/x/tools/internal/lsp.(*Server).didOpen(0xc00021c2a0, 0x17e3180, 0xc000218780, 0xc000214740, 0x0, 0xc000236140) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/text_synchronization.go:32 +0x183 golang.org/x/tools/internal/lsp.(*Server).DidOpen(0xc00021c2a0, 0x17e3180, 0xc000218780, 0xc000214740, 0xc000214740, 0x0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/server.go:134 +0x49 golang.org/x/tools/internal/lsp/protocol.serverHandler.Deliver(0x17f4d20, 0xc00021c2a0, 0x17e3180, 0xc000218780, 0xc0002187c0, 0x0, 0xc000048730) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/protocol/tsserver.go:113 +0x1cc7 golang.org/x/tools/internal/jsonrpc2.(*Conn).Run.func1(0xc0001e0240, 0xc0002187c0, 0xc00021c300, 0x17e3180, 0xc000218780, 0x0, 0x0, 0xc0000adad0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/jsonrpc2/jsonrpc2.go:370 +0x13a created by golang.org/x/tools/internal/jsonrpc2.(*Conn).Run 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/jsonrpc2/jsonrpc2.go:354 +0x8d8 isotope=% !go gopls -debug localhost:8081 -listen localhost:8080 serve  panic: runtime error: index out of range  goroutine 38 [running]: golang.org/x/tools/internal/lsp/cache.(*session).bestView(0xc000129900, 0xc0001ba600, 0x57, 0x57, 0x1c5f300) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/cache/session.go:172 +0x198 golang.org/x/tools/internal/lsp/cache.(*session).ViewOf(0xc000129900, 0xc0001ba600, 0x57, 0x0, 0x0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/cache/session.go:140 +0xdd golang.org/x/tools/internal/lsp.(*Server).didOpen(0xc000234240, 0x17e3180, 0xc000278240, 0xc000252040, 0x0, 0xc00024a1e0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/text_synchronization.go:32 +0x183 golang.org/x/tools/internal/lsp.(*Server).DidOpen(0xc000234240, 0x17e3180, 0xc000278240, 0xc000252040, 0xc000252040, 0x0) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/server.go:134 +0x49 golang.org/x/tools/internal/lsp/protocol.serverHandler.Deliver(0x17f4d20, 0xc000234240, 0x17e3180, 0xc000278240, 0xc000278280, 0x0, 0xc000046730) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/lsp/protocol/tsserver.go:113 +0x1cc7 golang.org/x/tools/internal/jsonrpc2.(*Conn).Run.func1(0xc0001ba5a0, 0xc000278280, 0xc0002342a0, 0x17e3180, 0xc000278240, 0x0, 0x0, 0xc000276d90) 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/jsonrpc2/jsonrpc2.go:370 +0x13a created by golang.org/x/tools/internal/jsonrpc2.(*Conn).Run 	/Users/marius/go/pkg/mod/golang.org/x/tools@v0.0.0-20190910165522-3cd124fa3ebb/internal/jsonrpc2/jsonrpc2.go:354 +0x8d8

This appears to be caused by this recent change. Gopls works fine with acme-lsp on the commit prior.

@gopherbot gopherbot added this to the Unreleased milestone Sep 10, 2019
@gopherbot
Copy link
Contributor

Thank you for filing a gopls issue! Please take a look at the Troubleshooting section of the gopls Wiki page, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Sep 10, 2019
@ianthehat
Copy link

Looks like acme-lsp is sending a didopen call before initialized, which it is not allowed to do.
We would have survived that before, although there would have been subtle bugs.
There is still a bug we should fix here (crashes are never ok) but if I am right then the fix is to start obeying the spec and rejecting all messages before the initialize handshake is done, in which case acme-lsp is still going to be broken until that is fixed as well.

@mariusae
Copy link
Author

Thanks, I'll take this over to the acme-lsp issue.

fhs added a commit to 9fans/acme-lsp that referenced this issue Sep 11, 2019
We need to send a `initialized` notification after we receive the result
of `initialize`. The spec doesn't mention anything about this in the
[initialize](https://microsoft.github.io/language-server-protocol/specification#initialize)
section, but it's documented here:
https://microsoft.github.io/language-server-protocol/specification#initialized

Fixes #16
Update golang/go#34212
@fhs
Copy link
Contributor

fhs commented Sep 11, 2019

acme-lsp wasn't sending the initialized notification after the initialize request. The spec isn't exactly very clear. There is no mention of the notification in the initialize request sections:

Until the server has responded to the initialize request with an InitializeResult, the client must not send any additional requests or notifications to the server. In addition the server is not allowed to send any requests or notifications to the client until it has responded with an InitializeResult, with the exception ...

@golang golang locked and limited conversation to collaborators Sep 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls.
Projects
None yet
Development

No branches or pull requests

4 participants