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: Shutdown does not interrupt current "work" #45476

Open
myitcv opened this issue Apr 9, 2021 · 1 comment
Open

x/tools/gopls: Shutdown does not interrupt current "work" #45476

myitcv opened this issue Apr 9, 2021 · 1 comment
Labels
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.

Comments

@myitcv
Copy link
Member

myitcv commented Apr 9, 2021

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

$ go version
go version devel +135c9f45ec Wed Mar 31 04:13:44 2021 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.1.1-0.20210330233337-8c34cc9cafff
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20210330233337-8c34cc9cafff

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
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +135c9f45ec Wed Mar 31 04:13:44 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3492538628=/tmp/go-build -gno-record-gcc-switches"

What did you do?

A recent fix in govim means that when Vim shuts down, it triggers a shutdown sequence as follows:

  1. User types :q (or equivalent)
  2. Vim send govim a VimLeave event, which fires before exiting Vim, and blocks on the processing of this event
  3. govim does some of its own tidy up
  4. govim calls the gopls.Shutdown method, blocking on its return
  5. govim does some more tidying up before finally returning to Vim
  6. With the VimLeave event now processed, Vim exits

This fix ensures that govim and gopls properly tidy up (temporary files) after themselves.

However, this appears to have highlighted an issue: that calling Shutdown does not interrupt existing "work" that gopls might be doing. Two things point to this being the case:

  • Per x/tools/gopls: scaling multi-module workspaces #41558 (comment), Vim does not exit until such time as the initial package load has completed (the Shutdown call appears to be blocked whilst the package load is happening)
  • As reported by @mvdan on Slack, "work" triggered off the back of a file change "storm" also cannot be interrupted, i.e. Vim does not exit until all the file change notifications and corresponding work have been handled.

As noted on Slack, in the case of the second issue there are many other issues at play, but it would still seem, like in the first case, desirable for Shutdown to be able to interrupt anything that gopls is doing.

For now we have a workaround in govim, to set a deadline of 500ms for the Shutdown call to be processed. The impact of this being that if the call to Shutdown exceed the deadline, gopls' temporary files (and other things?) are not tidied up.

What did you expect to see?

Shutdown being handled immediately, and not blocking on current "work".

What did you see instead?

As above.


cc @stamblerre

FYI @leitzler @mvdan

@myitcv myitcv 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. labels Apr 9, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 9, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 9, 2021
@stamblerre stamblerre modified the milestones: Unreleased, gopls/unplanned Apr 9, 2021
@stamblerre
Copy link
Contributor

Thanks for reporting this. We agree that this is an issue, but we probably won't be able to prioritize work on this in the near future.

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

No branches or pull requests

3 participants