Skip to content

Commit c90f89f

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> (cherry picked from commit db903f3) Reviewed-on: https://go-review.googlesource.com/c/tools/+/210203
1 parent 5a11726 commit c90f89f

File tree

1 file changed

+12
-1
lines changed

1 file changed

+12
-1
lines changed

internal/lsp/cache/snapshot.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,17 @@ func (s *snapshot) getIDs(uri span.URI) []packageID {
231231
return s.ids[uri]
232232
}
233233

234+
func (s *snapshot) getFileURIs() []span.URI {
235+
s.mu.Lock()
236+
defer s.mu.Unlock()
237+
238+
var uris []span.URI
239+
for uri := range s.files {
240+
uris = append(uris, uri)
241+
}
242+
return uris
243+
}
244+
234245
func (s *snapshot) getFile(uri span.URI) source.FileHandle {
235246
s.mu.Lock()
236247
defer s.mu.Unlock()
@@ -348,7 +359,7 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source
348359
// of all of the files in the same directory as this one.
349360
// TODO(rstambler): Speed this up by mapping directories to filenames.
350361
if dirStat, err := os.Stat(dir(f.URI().Filename())); err == nil {
351-
for uri := range v.snapshot.files {
362+
for _, uri := range v.snapshot.getFileURIs() {
352363
if fdirStat, err := os.Stat(dir(uri.Filename())); err == nil {
353364
if os.SameFile(dirStat, fdirStat) {
354365
for _, id := range v.snapshot.ids[uri] {

0 commit comments

Comments
 (0)