Skip to content

Commit 902dc27

Browse files
committed
go/token: clear cache after grabbing the mutex in RemoveFile
This fixes a race, that was possible to hit in RemoveFile. The file cache could have been populated concurrently just before grabbing of the mutex. Change-Id: I6a6a696452d8bd79ac4ac6574297390978353444 Reviewed-on: https://go-review.googlesource.com/c/go/+/705756 Reviewed-by: Junyang Shao <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Alan Donovan <[email protected]>
1 parent a13d085 commit 902dc27

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

src/go/token/position.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,11 +531,11 @@ func (s *FileSet) AddExistingFiles(files ...*File) {
531531
//
532532
// Removing a file that does not belong to the set has no effect.
533533
func (s *FileSet) RemoveFile(file *File) {
534-
s.last.CompareAndSwap(file, nil) // clear last file cache
535-
536534
s.mutex.Lock()
537535
defer s.mutex.Unlock()
538536

537+
s.last.CompareAndSwap(file, nil) // clear last file cache
538+
539539
pn, _ := s.tree.locate(file.key())
540540
if *pn != nil && (*pn).file == file {
541541
s.tree.delete(pn)

src/go/token/position_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,3 +579,45 @@ func fsetString(fset *FileSet) string {
579579
buf.WriteRune('}')
580580
return buf.String()
581581
}
582+
583+
// Test that File() does not return the already removed file, while used concurrently.
584+
func TestRemoveFileRace(t *testing.T) {
585+
fset := NewFileSet()
586+
587+
// Create bunch of files.
588+
var files []*File
589+
for i := range 20000 {
590+
f := fset.AddFile("f", -1, (i+1)*10)
591+
files = append(files, f)
592+
}
593+
594+
// governor goroutine
595+
race1, race2 := make(chan *File), make(chan *File)
596+
start := make(chan struct{})
597+
go func() {
598+
for _, f := range files {
599+
<-start
600+
race1 <- f
601+
race2 <- f
602+
}
603+
<-start // unlock main test goroutine
604+
close(race1)
605+
close(race2)
606+
}()
607+
608+
go func() {
609+
for f := range race1 {
610+
fset.File(Pos(f.Base()) + 5) // populates s.last with f
611+
}
612+
}()
613+
614+
start <- struct{}{}
615+
for f := range race2 {
616+
fset.RemoveFile(f)
617+
got := fset.File(Pos(f.Base()) + 5)
618+
if got != nil {
619+
t.Fatalf("file was not removed correctly")
620+
}
621+
start <- struct{}{}
622+
}
623+
}

0 commit comments

Comments
 (0)