From 4124316da0c55abb58bd81da62d6424cc92f7e59 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 3 Nov 2023 15:13:01 -0400 Subject: [PATCH] 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 Reviewed-by: Alan Donovan --- gopls/internal/lsp/cache/session.go | 3 +-- gopls/internal/lsp/cache/snapshot.go | 7 ++++--- gopls/internal/lsp/cache/view.go | 6 +----- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index 47b69af8bc5..e565f19e1a6 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -118,7 +118,6 @@ func (s *Session) createView(ctx context.Context, info *workspaceInformation, fo 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 @@ func (s *Session) createView(ctx context.Context, info *workspaceInformation, fo 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 ba1322d2ea6..47dd1ab714f 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -45,6 +45,7 @@ import ( "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 @@ func inVendor(uri span.URI) bool { 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 2565151a4df..246669c3290 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -52,10 +52,6 @@ type View struct { 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 @@ func (v *View) invalidateContent(ctx context.Context, changes map[span.URI]sourc 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")