go/token: use fine-grained locking in FileSet

Before, all accesses to the lines and infos tables of each File were
serialized by the lock of the owning FileSet, causing parsers running
in parallel to contend.  Now, each File has its own mutex.

This fixes a data race in (*File).PositionFor, which used to call
f.position then f.unpack without holding the mutex's lock.

Fixes golang/go#18348

Change-Id: Iaa5989b2eba88a7fb2e91c1a0a8bc1e7f6497f2b
Reviewed-on: https://go-review.googlesource.com/34591
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/go/token/position.go b/src/go/token/position.go
index 8442b270dbe19bd0be9928f48a4de663a3298a54..88d74168a1b4f1957e6c3b7d1c73177793be8346 100644 (file)
--- a/src/go/token/position.go
+++ b/src/go/token/position.go
@@ -94,7 +94,8 @@ type File struct {
        base int    // Pos value range for this file is [base...base+size]
        size int    // file size as provided to AddFile
 
-       // lines and infos are protected by set.mutex
+       // lines and infos are protected by mutex
+       mutex sync.Mutex
        lines []int // lines contains the offset of the first character for each line (the first entry is always 0)
        infos []lineInfo
 }
@@ -116,9 +117,9 @@ func (f *File) Size() int {
 
 // LineCount returns the number of lines in file f.
 func (f *File) LineCount() int {
-       f.set.mutex.RLock()
+       f.mutex.Lock()
        n := len(f.lines)
-       f.set.mutex.RUnlock()
+       f.mutex.Unlock()
        return n
 }
 
@@ -127,11 +128,11 @@ func (f *File) LineCount() int {
 // and smaller than the file size; otherwise the line offset is ignored.
 //
 func (f *File) AddLine(offset int) {
-       f.set.mutex.Lock()
+       f.mutex.Lock()
        if i := len(f.lines); (i == 0 || f.lines[i-1] < offset) && offset < f.size {
                f.lines = append(f.lines, offset)
        }
-       f.set.mutex.Unlock()
+       f.mutex.Unlock()
 }
 
 // MergeLine merges a line with the following line. It is akin to replacing
@@ -143,8 +144,8 @@ func (f *File) MergeLine(line int) {
        if line <= 0 {
                panic("illegal line number (line numbering starts at 1)")
        }
-       f.set.mutex.Lock()
-       defer f.set.mutex.Unlock()
+       f.mutex.Lock()
+       defer f.mutex.Unlock()
        if line >= len(f.lines) {
                panic("illegal line number")
        }
@@ -176,9 +177,9 @@ func (f *File) SetLines(lines []int) bool {
        }
 
        // set lines table
-       f.set.mutex.Lock()
+       f.mutex.Lock()
        f.lines = lines
-       f.set.mutex.Unlock()
+       f.mutex.Unlock()
        return true
 }
 
@@ -198,9 +199,9 @@ func (f *File) SetLinesForContent(content []byte) {
        }
 
        // set lines table
-       f.set.mutex.Lock()
+       f.mutex.Lock()
        f.lines = lines
-       f.set.mutex.Unlock()
+       f.mutex.Unlock()
 }
 
 // A lineInfo object describes alternative file and line number
@@ -222,11 +223,11 @@ type lineInfo struct {
 // information for //line filename:line comments in source files.
 //
 func (f *File) AddLineInfo(offset int, filename string, line int) {
-       f.set.mutex.Lock()
+       f.mutex.Lock()
        if i := len(f.infos); i == 0 || f.infos[i-1].Offset < offset && offset < f.size {
                f.infos = append(f.infos, lineInfo{offset, filename, line})
        }
-       f.set.mutex.Unlock()
+       f.mutex.Unlock()
 }
 
 // Pos returns the Pos value for the given file offset;
@@ -267,6 +268,8 @@ func searchLineInfos(a []lineInfo, x int) int {
 // possibly adjusted by //line comments; otherwise those comments are ignored.
 //
 func (f *File) unpack(offset int, adjusted bool) (filename string, line, column int) {
+       f.mutex.Lock()
+       defer f.mutex.Unlock()
        filename = f.name
        if i := searchInts(f.lines, offset); i >= 0 {
                line, column = i+1, offset-f.lines[i]+1
@@ -371,7 +374,7 @@ func (s *FileSet) AddFile(filename string, base, size int) *File {
                panic("illegal base or size")
        }
        // base >= s.base && size >= 0
-       f := &File{s, filename, base, size, []int{0}, nil}
+       f := &File{set: s, name: filename, base: base, size: size, lines: []int{0}}
        base += size + 1 // +1 because EOF also has a position
        if base < 0 {
                panic("token.Pos offset overflow (> 2G of source code in file set)")
@@ -446,9 +449,7 @@ func (s *FileSet) File(p Pos) (f *File) {
 func (s *FileSet) PositionFor(p Pos, adjusted bool) (pos Position) {
        if p != NoPos {
                if f := s.file(p); f != nil {
-                       s.mutex.RLock()
-                       pos = f.position(p, adjusted)
-                       s.mutex.RUnlock()
+                       return f.position(p, adjusted)
                }
        }
        return

diff --git a/src/go/token/serialize.go b/src/go/token/serialize.go
index 4adc8f9e33422f656039c98a50c65713b3734698..d0ea34517a442fd25ac11751179147d1ea6fd074 100644 (file)
--- a/src/go/token/serialize.go
+++ b/src/go/token/serialize.go
@@ -30,7 +30,14 @@ func (s *FileSet) Read(decode func(interface{}) error) error {
        files := make([]*File, len(ss.Files))
        for i := 0; i < len(ss.Files); i++ {
                f := &ss.Files[i]
-               files[i] = &File{s, f.Name, f.Base, f.Size, f.Lines, f.Infos}
+               files[i] = &File{
+                       set:   s,
+                       name:  f.Name,
+                       base:  f.Base,
+                       size:  f.Size,
+                       lines: f.Lines,
+                       infos: f.Infos,
+               }
        }
        s.files = files
        s.last = nil
@@ -47,7 +54,15 @@ func (s *FileSet) Write(encode func(interface{}) error) error {
        ss.Base = s.base
        files := make([]serializedFile, len(s.files))
        for i, f := range s.files {
-               files[i] = serializedFile{f.name, f.base, f.size, f.lines, f.infos}
+               f.mutex.Lock()
+               files[i] = serializedFile{
+                       Name:  f.name,
+                       Base:  f.base,
+                       Size:  f.size,
+                       Lines: append([]int(nil), f.lines...),
+                       Infos: append([]lineInfo(nil), f.infos...),
+               }
+               f.mutex.Unlock()
        }
        ss.Files = files
        s.mutex.Unlock()