From db903f390e86fb3c8432384267eb272105f29479 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 5 Dec 2019 19:17:56 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/snapshot.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 0ebfc70c45a..275893b6cb9 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -424,6 +424,17 @@ func (s *snapshot) getIDs(uri span.URI) []packageID { 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 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source // 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] {