From 52fcff3ec147ea8ae48c023f3d5000a8bf42fe8c Mon Sep 17 00:00:00 2001 From: Jan Mercl <0xjnml@gmail.com> Date: Tue, 2 Aug 2016 13:00:46 +0200 Subject: [PATCH] go/token: Fix race in FileSet.PositionFor. Methods of FileSet are documented to be safe for concurrent use by multiple goroutines, so FileSet is protected by a mutex and all its methods use it to prevent concurrent mutations. All methods of File that mutate the respective FileSet, including AddLine, do also lock its mutex, but that does not help when PositionFor is invoked concurrently and reads without synchronization what AddLine mutates. The change adds acquiring a RLock around the racy call of File.position and the respective test. Fixes #16548 Change-Id: Iecaaa02630b2532cb29ab555376633ee862315dd Reviewed-on: https://go-review.googlesource.com/25345 Reviewed-by: Robert Griesemer Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/go/token/position.go | 2 ++ src/go/token/position_test.go | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/go/token/position.go b/src/go/token/position.go index 7306083b0d..d4171d80e0 100644 --- a/src/go/token/position.go +++ b/src/go/token/position.go @@ -446,7 +446,9 @@ 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 diff --git a/src/go/token/position_test.go b/src/go/token/position_test.go index d26939ce27..63984bc872 100644 --- a/src/go/token/position_test.go +++ b/src/go/token/position_test.go @@ -214,7 +214,7 @@ func TestFileSetCacheUnlikely(t *testing.T) { } } -// issue 4345. Test concurrent use of FileSet.Pos does not trigger a +// issue 4345. Test that concurrent use of FileSet.Pos does not trigger a // race in the FileSet position cache. func TestFileSetRace(t *testing.T) { fset := NewFileSet() @@ -237,6 +237,35 @@ func TestFileSetRace(t *testing.T) { stop.Wait() } +// issue 16548. Test that concurrent use of File.AddLine and FileSet.PositionFor +// does not trigger a race in the FileSet position cache. +func TestFileSetRace2(t *testing.T) { + const N = 1e3 + var ( + fset = NewFileSet() + file = fset.AddFile("", -1, N) + ch = make(chan int, 2) + ) + + go func() { + for i := 0; i < N; i++ { + file.AddLine(i) + } + ch <- 1 + }() + + go func() { + pos := file.Pos(0) + for i := 0; i < N; i++ { + fset.PositionFor(pos, false) + } + ch <- 1 + }() + + <-ch + <-ch +} + func TestPositionFor(t *testing.T) { src := []byte(` foo -- 2.48.1