gopls/internal/lsp/cache: fix data race in Symbols
We forgot to acquire a lock while accessing snapshot.files.
Also, upgrade errgroup and use its new SetLimit feature.
Change-Id: Iff1449fd727f0766f6c81391acccaa21951f59be
Reviewed-on: https://go-review.googlesource.com/c/tools/+/452058
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index ae5fe28..c2cb946 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1094,32 +1094,37 @@
// Symbols extracts and returns the symbols for each file in all the snapshot's views.
func (s *snapshot) Symbols(ctx context.Context) map[span.URI][]source.Symbol {
+ // Read the set of Go files out of the snapshot.
+ var goFiles []source.VersionedFileHandle
+ s.mu.Lock()
+ s.files.Range(func(uri span.URI, f source.VersionedFileHandle) {
+ if s.View().FileKind(f) == source.Go {
+ goFiles = append(goFiles, f)
+ }
+ })
+ s.mu.Unlock()
+
+ // Symbolize them in parallel.
var (
group errgroup.Group
- nprocs = 2 * runtime.GOMAXPROCS(-1) // symbolize is a mix of I/O and CPU
- iolimit = make(chan struct{}, nprocs) // I/O limiting counting semaphore
+ nprocs = 2 * runtime.GOMAXPROCS(-1) // symbolize is a mix of I/O and CPU
resultMu sync.Mutex
result = make(map[span.URI][]source.Symbol)
)
- s.files.Range(func(uri span.URI, f source.VersionedFileHandle) {
- if s.View().FileKind(f) != source.Go {
- return // workspace symbols currently supports only Go files.
- }
-
- // TODO(adonovan): upgrade errgroup and use group.SetLimit(nprocs).
- iolimit <- struct{}{} // acquire token
+ group.SetLimit(nprocs)
+ for _, f := range goFiles {
+ f := f
group.Go(func() error {
- defer func() { <-iolimit }() // release token
symbols, err := s.symbolize(ctx, f)
if err != nil {
return err
}
resultMu.Lock()
- result[uri] = symbols
+ result[f.URI()] = symbols
resultMu.Unlock()
return nil
})
- })
+ }
// Keep going on errors, but log the first failure.
// Partial results are better than no symbol results.
if err := group.Wait(); err != nil {
