internal/lsp: fix race condition in caching
This change fixes a race condition in the metadata caching logic.
Also, some minor fixes to comments and invalidation logic (it's not
necessary to invalidate ASTs when a package is invalidated).
Change-Id: I927bf6fbc661a86ef0ba99b29a9ed979cd1eb95d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 56ffde2..43018ca 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -229,10 +229,13 @@
log.Error(ctx, "not a Go file", nil, telemetry.File.Of(filename))
continue
}
+ // Cache the metadata for this file.
+ gof.mu.Lock()
if gof.meta == nil {
gof.meta = make(map[packageID]*metadata)
}
gof.meta[m.id] = m
+ gof.mu.Unlock()
}
// Preserve the import graph.
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index ee830d3..3da4a6f 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/telemetry"
+ "golang.org/x/tools/internal/lsp/telemetry/log"
"golang.org/x/tools/internal/lsp/telemetry/trace"
"golang.org/x/tools/internal/memoize"
errors "golang.org/x/xerrors"
@@ -105,7 +106,7 @@
ioLimit <- struct{}{}
buf, _, err := fh.Read(ctx)
- <-ioLimit // Make sure to release the token, even when an error is returned.
+ <-ioLimit
if err != nil {
return nil, err
}
@@ -122,7 +123,7 @@
// Fix any badly parsed parts of the AST.
tok := c.fset.File(ast.Pos())
if err := fix(ctx, ast, tok, buf); err != nil {
- // TODO: Do something with the error (need access to a logger in here).
+ log.Error(ctx, "failed to fix AST", err)
}
}
if ast == nil {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e4db451..d4ff267 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -409,13 +409,6 @@
files: hashParseKeys(cph.Files()),
config: hashConfig(cph.Config()),
})
- // Also, delete all of the cached ParseGoHandles.
- for _, ph := range cph.Files() {
- v.session.cache.store.Delete(parseKey{
- file: ph.File().Identity(),
- mode: ph.Mode(),
- })
- }
}
delete(gof.pkgs, id)
gof.mu.Unlock()
