1
0
mirror of https://github.com/golang/go synced 2024-11-18 16:54:43 -07:00

internal/lsp: fix race condition in type-checking

Change-Id: Idff7cc3a28b4a03ce0d374c6c5035b51330143b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/178724
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Rebecca Stambler 2019-05-24 16:03:39 -04:00
parent d487f80763
commit d532c0723d

View File

@ -51,7 +51,7 @@ func (v *view) parse(ctx context.Context, f *goFile) ([]packages.Error, error) {
go imp.Import(importPath)
}
// Type-check package.
pkg, err := imp.typeCheck(f.meta.pkgPath)
pkg, err := imp.getPkg(f.meta.pkgPath)
if pkg == nil || pkg.GetTypes() == nil {
return nil, err
}
@ -181,6 +181,14 @@ type importer struct {
}
func (imp *importer) Import(pkgPath string) (*types.Package, error) {
pkg, err := imp.getPkg(pkgPath)
if err != nil {
return nil, err
}
return pkg.types, nil
}
func (imp *importer) getPkg(pkgPath string) (*pkg, error) {
if _, ok := imp.seen[pkgPath]; ok {
return nil, fmt.Errorf("circular import detected")
}
@ -205,7 +213,7 @@ func (imp *importer) Import(pkgPath string) (*types.Package, error) {
if e.err != nil {
return nil, e.err
}
return e.pkg.types, nil
return e.pkg, nil
}
func (imp *importer) typeCheck(pkgPath string) (*pkg, error) {
@ -266,32 +274,32 @@ func (imp *importer) typeCheck(pkgPath string) (*pkg, error) {
check.Files(pkg.syntax)
// Add every file in this package to our cache.
imp.view.cachePackage(imp.ctx, pkg, meta)
imp.cachePackage(imp.ctx, pkg, meta)
return pkg, nil
}
func (v *view) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) {
func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) {
for _, file := range pkg.GetSyntax() {
// TODO: If a file is in multiple packages, which package do we store?
if !file.Pos().IsValid() {
v.Session().Logger().Errorf(ctx, "invalid position for file %v", file.Name)
imp.view.Session().Logger().Errorf(ctx, "invalid position for file %v", file.Name)
continue
}
tok := v.Session().Cache().FileSet().File(file.Pos())
tok := imp.view.Session().Cache().FileSet().File(file.Pos())
if tok == nil {
v.Session().Logger().Errorf(ctx, "no token.File for %v", file.Name)
imp.view.Session().Logger().Errorf(ctx, "no token.File for %v", file.Name)
continue
}
fURI := span.FileURI(tok.Name())
f, err := v.getFile(fURI)
f, err := imp.view.getFile(fURI)
if err != nil {
v.Session().Logger().Errorf(ctx, "no file: %v", err)
imp.view.Session().Logger().Errorf(ctx, "no file: %v", err)
continue
}
gof, ok := f.(*goFile)
if !ok {
v.Session().Logger().Errorf(ctx, "not a go file: %v", f.URI())
imp.view.Session().Logger().Errorf(ctx, "not a go file: %v", f.URI())
continue
}
gof.token = tok
@ -300,26 +308,15 @@ func (v *view) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) {
gof.pkg = pkg
}
v.pcache.mu.Lock()
defer v.pcache.mu.Unlock()
// Cache the entry for this package.
// All dependencies are cached through calls to *imp.Import.
e := &entry{
pkg: pkg,
err: nil,
ready: make(chan struct{}),
}
close(e.ready)
v.pcache.packages[pkg.pkgPath] = e
// Set imports of package to correspond to cached packages.
// We lock the package cache, but we shouldn't get any inconsistencies
// because we are still holding the lock on the view.
for importPath := range meta.children {
if importEntry, ok := v.pcache.packages[importPath]; ok {
pkg.imports[importPath] = importEntry.pkg
importPkg, err := imp.getPkg(importPath)
if err != nil {
continue
}
pkg.imports[importPath] = importPkg
}
}