Skip to content

Commit

Permalink
fix #302: fix concurrent astcache access
Browse files Browse the repository at this point in the history
  • Loading branch information
jirfag committed Nov 24, 2018
1 parent dba3907 commit 255a39b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 28 deletions.
48 changes: 27 additions & 21 deletions pkg/lint/astcache/astcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"go/parser"
"go/token"
"path/filepath"
"strings"
"time"

"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -33,37 +32,30 @@ func NewCache(log logutils.Log) *Cache {
}
}

func (c Cache) Get(filename string) *File {
return c.m[filepath.Clean(filename)]
}

func (c Cache) keys() []string {
func (c Cache) ParsedFilenames() []string {
var keys []string
for k := range c.m {
keys = append(keys, k)
}
return keys
}

func (c Cache) GetOrParse(filename string, fset *token.FileSet) *File {
if !filepath.IsAbs(filename) {
absFilename, err := filepath.Abs(filename)
if err != nil {
c.log.Warnf("Can't abs-ify filename %s: %s", filename, err)
} else {
filename = absFilename
}
func (c Cache) normalizeFilename(filename string) string {
if filepath.IsAbs(filename) {
return filepath.Clean(filename)
}

f := c.m[filename]
if f != nil {
return f
absFilename, err := filepath.Abs(filename)
if err != nil {
c.log.Warnf("Can't abs-ify filename %s: %s", filename, err)
return filename
}

c.log.Infof("Parse AST for file %s on demand, existing files are %s",
filename, strings.Join(c.keys(), ","))
c.parseFile(filename, fset)
return c.m[filename]
return absFilename
}

func (c Cache) Get(filename string) *File {
return c.m[c.normalizeFilename(filename)]
}

func (c Cache) GetAllValidFiles() []*File {
Expand All @@ -81,6 +73,18 @@ func (c *Cache) prepareValidFiles() {
c.s = files
}

func LoadFromFilenames(log logutils.Log, filenames ...string) *Cache {
c := NewCache(log)

fset := token.NewFileSet()
for _, filename := range filenames {
c.parseFile(filename, fset)
}

c.prepareValidFiles()
return c
}

func LoadFromPackages(pkgs []*packages.Package, log logutils.Log) (*Cache, error) {
c := NewCache(log)

Expand Down Expand Up @@ -127,6 +131,8 @@ func (c *Cache) parseFile(filePath string, fset *token.FileSet) {
fset = token.NewFileSet()
}

filePath = c.normalizeFilename(filePath)

// comments needed by e.g. golint
f, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments)
c.m[filePath] = &File{
Expand Down
6 changes: 3 additions & 3 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ func (p *AutogeneratedExclude) getOrCreateFileSummary(i *result.Issue) (*ageFile
return nil, fmt.Errorf("no file path for issue")
}

f := p.astCache.GetOrParse(i.FilePath(), nil)
if f.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), f.Err)
f := p.astCache.Get(i.FilePath())
if f == nil || f.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %v", i.FilePath(), f)
}

autogenDebugf("file %q: astcache file is %+v", i.FilePath(), *f)
Expand Down
6 changes: 3 additions & 3 deletions pkg/result/processors/nolint.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) {
return nil, fmt.Errorf("no file path for issue")
}

file := p.astCache.GetOrParse(i.FilePath(), nil)
if file.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), file.Err)
file := p.astCache.Get(i.FilePath())
if file == nil || file.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %v, astcache is %v", i.FilePath(), file, p.astCache.ParsedFilenames())
}

fd.ignoredRanges = p.buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath())
Expand Down
7 changes: 6 additions & 1 deletion pkg/result/processors/nolint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ func newNolint2FileIssue(line int) result.Issue {
}

func newTestNolintProcessor(log logutils.Log) *Nolint {
return NewNolint(astcache.NewCache(log), log)
cache := astcache.LoadFromFilenames(log,
filepath.Join("testdata", "nolint.go"),
filepath.Join("testdata", "nolint2.go"),
filepath.Join("testdata", "nolint_bad_names.go"),
)
return NewNolint(cache, log)
}

func getOkLogger(ctrl *gomock.Controller) *logutils.MockLog {
Expand Down

0 comments on commit 255a39b

Please sign in to comment.