Skip to content

Commit 3da6f18

Browse files
authored
Reuse source.Opener and ir.Session to fix executor memory leaks in LSP (#4230)
1 parent 3a5f466 commit 3da6f18

File tree

3 files changed

+48
-10
lines changed

3 files changed

+48
-10
lines changed

private/buf/buflsp/buflsp.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import (
3030
"github.com/bufbuild/buf/private/pkg/storage"
3131
"github.com/bufbuild/buf/private/pkg/wasm"
3232
"github.com/bufbuild/protocompile/experimental/incremental"
33+
"github.com/bufbuild/protocompile/experimental/ir"
34+
"github.com/bufbuild/protocompile/experimental/source"
3335
"go.lsp.dev/jsonrpc2"
3436
"go.lsp.dev/protocol"
3537
"go.uber.org/zap"
@@ -60,6 +62,8 @@ func Serve(
6062
wasmRuntime: wasmRuntime,
6163
wktBucket: wktBucket,
6264
queryExecutor: queryExecutor,
65+
opener: source.NewMap(nil),
66+
irSession: new(ir.Session),
6367
}
6468
lsp.fileManager = newFileManager(lsp)
6569
lsp.workspaceManager = newWorkspaceManager(lsp)
@@ -90,6 +94,8 @@ type lsp struct {
9094
fileManager *fileManager
9195
workspaceManager *workspaceManager
9296
queryExecutor *incremental.Executor
97+
opener source.Map
98+
irSession *ir.Session
9399
wktBucket storage.ReadBucket
94100
shutdown bool
95101

private/buf/buflsp/file.go

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ type file struct {
6262
objectInfo storage.ObjectInfo // Info in the context of the workspace.
6363

6464
ir *ir.File
65+
irQuery queries.IR
66+
fileQuery queries.File
6567
referenceableSymbols map[ir.FullName]*symbol
6668
referenceSymbols []*symbol
6769
symbols []*symbol
@@ -97,6 +99,11 @@ func (f *file) Reset(ctx context.Context) {
9799
f.workspace.Release()
98100
f.workspace = nil
99101
}
102+
// Evict the query key if there is a query cached on the file. We cache the [queries.File]
103+
// query since this allows the executor to evict all dependent queries, e.g. AST and IR.
104+
if f.fileQuery.Path != "" {
105+
f.lsp.queryExecutor.Evict(f.fileQuery.Key())
106+
}
100107
// Clear the file as nothing should use it after reset. This asserts that.
101108
*f = file{}
102109
}
@@ -229,21 +236,46 @@ func (f *file) RefreshIR(ctx context.Context) {
229236
// Opener creates a cached view of all files in the workspace.
230237
pathToFiles := f.workspace.PathToFile()
231238
files := make([]*file, 0, len(pathToFiles))
232-
openerMap := make(map[string]*source.File, len(pathToFiles))
239+
var evictQueryKeys []any
240+
241+
openerMap := f.lsp.opener.Get()
233242
for path, file := range pathToFiles {
234-
openerMap[path] = file.file
243+
current := openerMap[path]
244+
// If there is no entry for the current path or if the file content has changed, we
245+
// update the opener and set a new query.
246+
if current == nil || current.Text() != file.file.Text() {
247+
openerMap[path] = file.file
248+
// Add the query key for eviction if there is a query cached on the file. We cache
249+
// the [queries.File] query since this allows the executor to evict all dependent
250+
// queries, e.g. AST and IR.
251+
if file.fileQuery.Path != "" {
252+
evictQueryKeys = append(evictQueryKeys, file.fileQuery.Key())
253+
}
254+
file.fileQuery = queries.File{
255+
Opener: file.lsp.opener,
256+
Path: path,
257+
ReportError: true, // [queries.AST] sets this to be true.
258+
}
259+
file.irQuery = queries.IR{
260+
Opener: file.lsp.opener,
261+
Path: file.objectInfo.Path(),
262+
Session: file.lsp.irSession,
263+
}
264+
}
235265
files = append(files, file)
236266
}
237-
opener := source.NewMap(openerMap)
267+
// Remove paths that are no longer in the current workspace and evict stale query keys.
268+
for path := range openerMap {
269+
if _, ok := pathToFiles[path]; !ok {
270+
delete(openerMap, path)
271+
}
272+
}
273+
f.lsp.queryExecutor.Evict(evictQueryKeys...)
238274

239-
session := new(ir.Session)
240275
queries := xslices.Map(files, func(file *file) incremental.Query[*ir.File] {
241-
return queries.IR{
242-
Opener: opener,
243-
Path: file.objectInfo.Path(),
244-
Session: session,
245-
}
276+
return file.irQuery
246277
})
278+
247279
results, diagnosticReport, err := incremental.Run(
248280
ctx,
249281
f.lsp.queryExecutor,

private/buf/buflsp/workspace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func (w *workspace) indexFiles(ctx context.Context) {
254254
}
255255

256256
// Currently we only associate a file with one workspace. This assumption isn't accurate
257-
// for shared dependencies. Here we update to the lastest, most recently used, workspace.
257+
// for shared dependencies. Here we update to the latest, most recently used, workspace.
258258
// This will make goto definition and find references only work in that workspace.
259259
if oldWorkspace := file.workspace; oldWorkspace != nil && oldWorkspace != w {
260260
oldWorkspace.Release()

0 commit comments

Comments
 (0)