gopls/internal/lsp/cache: remove baseCtx from the View
Views don't do work, so should have no need of a context. Instead, chain
the contexts inside of clone.
Also, fix a latent bug where the context used in the view importsState
is the snapshot backgroundCtx rather than view baseCtx: the importsState
outlives the snapshot, so should not reference the snapshot context.
Fortunately, the context was thus far only used for logging.
For golang/go#57979
Change-Id: Icf55d69e82f19b3fd52ca7d9266df2b5589bb36e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539676
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go
index 47b69af..e565f19 100644
--- a/gopls/internal/lsp/cache/session.go
+++ b/gopls/internal/lsp/cache/session.go
@@ -118,7 +118,6 @@
folder: folder,
initialWorkspaceLoad: make(chan struct{}),
initializationSema: make(chan struct{}, 1),
- baseCtx: baseCtx,
moduleUpgrades: map[span.URI]map[string]string{},
vulns: map[span.URI]*vulncheck.Result{},
parseCache: s.parseCache,
@@ -126,7 +125,7 @@
workspaceInformation: info,
}
v.importsState = &importsState{
- ctx: backgroundCtx,
+ ctx: baseCtx,
processEnv: &imports.ProcessEnv{
GocmdRunner: s.gocmdRunner,
SkipPathInScan: func(dir string) bool {
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index ba1322d..47dd1ab 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -45,6 +45,7 @@
"golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/persistent"
"golang.org/x/tools/internal/typesinternal"
+ "golang.org/x/tools/internal/xcontext"
)
type snapshot struct {
@@ -1813,20 +1814,20 @@
return found && strings.Contains(after, "/")
}
-func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]source.FileHandle) (*snapshot, func()) {
+func (s *snapshot) clone(ctx context.Context, changes map[span.URI]source.FileHandle) (*snapshot, func()) {
ctx, done := event.Start(ctx, "cache.snapshot.clone")
defer done()
s.mu.Lock()
defer s.mu.Unlock()
- bgCtx, cancel := context.WithCancel(bgCtx)
+ backgroundCtx, cancel := context.WithCancel(event.Detach(xcontext.Detach(s.backgroundCtx)))
result := &snapshot{
sequenceID: s.sequenceID + 1,
globalID: nextSnapshotID(),
store: s.store,
view: s.view,
- backgroundCtx: bgCtx,
+ backgroundCtx: backgroundCtx,
cancel: cancel,
builtin: s.builtin,
initialized: s.initialized,
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index 2565151..246669c 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -52,10 +52,6 @@
gocmdRunner *gocommand.Runner // limits go command concurrency
- // baseCtx is the context handed to NewView. This is the parent of all
- // background contexts created for this view.
- baseCtx context.Context
-
folder *Folder
// Workspace information. The fields below are immutable, and together with
@@ -893,7 +889,7 @@
prevSnapshot.AwaitInitialized(ctx)
// Save one lease of the cloned snapshot in the view.
- v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, v.baseCtx, changes)
+ v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, changes)
prevReleaseSnapshot()
v.destroy(prevSnapshot, "View.invalidateContent")
