Skip to content

Commit 4124316

Browse files
committed
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 <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 8b89cfa commit 4124316

File tree

3 files changed

+6
-10
lines changed

3 files changed

+6
-10
lines changed

gopls/internal/lsp/cache/session.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,14 @@ func (s *Session) createView(ctx context.Context, info *workspaceInformation, fo
118118
folder: folder,
119119
initialWorkspaceLoad: make(chan struct{}),
120120
initializationSema: make(chan struct{}, 1),
121-
baseCtx: baseCtx,
122121
moduleUpgrades: map[span.URI]map[string]string{},
123122
vulns: map[span.URI]*vulncheck.Result{},
124123
parseCache: s.parseCache,
125124
fs: s.overlayFS,
126125
workspaceInformation: info,
127126
}
128127
v.importsState = &importsState{
129-
ctx: backgroundCtx,
128+
ctx: baseCtx,
130129
processEnv: &imports.ProcessEnv{
131130
GocmdRunner: s.gocmdRunner,
132131
SkipPathInScan: func(dir string) bool {

gopls/internal/lsp/cache/snapshot.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
"golang.org/x/tools/internal/packagesinternal"
4646
"golang.org/x/tools/internal/persistent"
4747
"golang.org/x/tools/internal/typesinternal"
48+
"golang.org/x/tools/internal/xcontext"
4849
)
4950

5051
type snapshot struct {
@@ -1813,20 +1814,20 @@ func inVendor(uri span.URI) bool {
18131814
return found && strings.Contains(after, "/")
18141815
}
18151816

1816-
func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]source.FileHandle) (*snapshot, func()) {
1817+
func (s *snapshot) clone(ctx context.Context, changes map[span.URI]source.FileHandle) (*snapshot, func()) {
18171818
ctx, done := event.Start(ctx, "cache.snapshot.clone")
18181819
defer done()
18191820

18201821
s.mu.Lock()
18211822
defer s.mu.Unlock()
18221823

1823-
bgCtx, cancel := context.WithCancel(bgCtx)
1824+
backgroundCtx, cancel := context.WithCancel(event.Detach(xcontext.Detach(s.backgroundCtx)))
18241825
result := &snapshot{
18251826
sequenceID: s.sequenceID + 1,
18261827
globalID: nextSnapshotID(),
18271828
store: s.store,
18281829
view: s.view,
1829-
backgroundCtx: bgCtx,
1830+
backgroundCtx: backgroundCtx,
18301831
cancel: cancel,
18311832
builtin: s.builtin,
18321833
initialized: s.initialized,

gopls/internal/lsp/cache/view.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ type View struct {
5252

5353
gocmdRunner *gocommand.Runner // limits go command concurrency
5454

55-
// baseCtx is the context handed to NewView. This is the parent of all
56-
// background contexts created for this view.
57-
baseCtx context.Context
58-
5955
folder *Folder
6056

6157
// 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
893889
prevSnapshot.AwaitInitialized(ctx)
894890

895891
// Save one lease of the cloned snapshot in the view.
896-
v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, v.baseCtx, changes)
892+
v.snapshot, v.releaseSnapshot = prevSnapshot.clone(ctx, changes)
897893

898894
prevReleaseSnapshot()
899895
v.destroy(prevSnapshot, "View.invalidateContent")

0 commit comments

Comments
 (0)