1
0
mirror of https://github.com/golang/go synced 2024-11-18 20:04:52 -07:00

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>
This commit is contained in:
Rebecca Stambler 2019-12-05 19:17:56 -05:00
parent 3393d29bb9
commit db903f390e

View File

@ -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] {