internal/lsp: fix concurrent map write in file invalidation
The invalidateContent function does not acquire a snapshot's mutex to
avoid blocking other work (even though it probably should since it's
only called after a context is canceled). A case was added to iterate
through files when a file is created, and it did not respect the fact
that the snapshot's mutex was not locked, resulting in a concurrent map
read and write. This change makes sure that the access of the snapshot's
files map is guarded by a mutex.
As a follow-up, we should just acquire snapshot.mu in invalidateContent.
Updates golang/go#36006
Change-Id: Idd904ae582055ce786062df50875ac7f0896fd1c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210199
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 0ebfc70..275893b 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -424,6 +424,17 @@
return s.ids[uri]
}
+func (s *snapshot) getFileURIs() []span.URI {
+ s.mu.Lock()
+ defer s.mu.Unlock()
+
+ var uris []span.URI
+ for uri := range s.files {
+ uris = append(uris, uri)
+ }
+ return uris
+}
+
func (s *snapshot) getFile(uri span.URI) source.FileHandle {
s.mu.Lock()
defer s.mu.Unlock()
@@ -556,7 +567,7 @@
// TODO(rstambler): Speed this up by mapping directories to filenames.
if action == source.Create {
if dirStat, err := os.Stat(dir(f.URI().Filename())); err == nil {
- for uri := range v.snapshot.files {
+ for _, uri := range v.snapshot.getFileURIs() {
if fdirStat, err := os.Stat(dir(uri.Filename())); err == nil {
if os.SameFile(dirStat, fdirStat) {
for _, id := range v.snapshot.ids[uri] {
