From 7bc3c281e1f3bf5d2318c91251fb1080c6eaaacb Mon Sep 17 00:00:00 2001
From: Rob Findley <rfindley@google.com>
Date: Mon, 9 Aug 2021 18:41:56 -0400
Subject: [PATCH] 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>
---
 internal/lsp/source/workspace_symbol.go | 44 +++++++++++++++----------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go
index 9172edf79b..ecea2b4ace 100644
--- a/internal/lsp/source/workspace_symbol.go
+++ b/internal/lsp/source/workspace_symbol.go
@@ -306,6 +306,7 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S
 
 	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 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S
 		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 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S
 				// 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++ {