internal/lsp/source: fix race in workspace symbols with multiple views
Unfortunately only after merging CL 338729 did I use it in a multi-view
workspace. That CL added a goroutine per matcher to scan symbols, but
unfortunately did this for each view, resulting in a race if there are
multiple views.
The fix is straightforward.
Change-Id: I405b37921883f9617f17c1e1506ff67b4c661cbc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340970
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go
index 9172edf..ecea2b4 100644
--- a/internal/lsp/source/workspace_symbol.go
+++ b/internal/lsp/source/workspace_symbol.go
@@ -306,6 +306,7 @@
results := make(chan *symbolStore)
matcherlen := len(sc.matchers)
+ files := make(map[span.URI]symbolFile)
for _, v := range views {
snapshot, release := v.Snapshot(ctx)
@@ -314,9 +315,12 @@
if err != nil {
return nil, err
}
- var work []symbolFile
for uri, syms := range psyms {
+ // Only scan each file once.
+ if _, ok := files[uri]; ok {
+ continue
+ }
mds, err := snapshot.MetadataForFile(ctx, uri)
if err != nil {
return nil, err
@@ -325,26 +329,30 @@
// TODO: should use the bug reporting API
continue
}
- work = append(work, symbolFile{uri, mds[0], syms})
+ files[uri] = symbolFile{uri, mds[0], syms}
}
+ }
- // Compute matches concurrently. Each symbolWorker has its own symbolStore,
- // which we merge at the end.
+ var work []symbolFile
+ for _, f := range files {
+ work = append(work, f)
+ }
- for i, matcher := range sc.matchers {
- go func(i int, matcher matcherFunc) {
- w := &symbolWorker{
- symbolizer: sc.symbolizer,
- matcher: matcher,
- ss: &symbolStore{},
- roots: roots,
- }
- for j := i; j < len(work); j += matcherlen {
- w.matchFile(work[j])
- }
- results <- w.ss
- }(i, matcher)
- }
+ // Compute matches concurrently. Each symbolWorker has its own symbolStore,
+ // which we merge at the end.
+ for i, matcher := range sc.matchers {
+ go func(i int, matcher matcherFunc) {
+ w := &symbolWorker{
+ symbolizer: sc.symbolizer,
+ matcher: matcher,
+ ss: &symbolStore{},
+ roots: roots,
+ }
+ for j := i; j < len(work); j += matcherlen {
+ w.matchFile(work[j])
+ }
+ results <- w.ss
+ }(i, matcher)
}
for i := 0; i < matcherlen; i++ {
