Merge pull request #134 from corentone/mem-races

mem/file.go - Fix some races in accessing fields of FileData

.travis.yml

@@ -16,5 +16,5 @@ matrix:
   fast_finish: true
 
 script:
-  - go test -v ./...
+  - go test -race -v ./...
   - go build

appveyor.yml

@@ -12,4 +12,4 @@ build_script:
 
     go build github.com/spf13/afero
 test_script:
-- cmd: go test -v github.com/spf13/afero
+- cmd: go test --race -v github.com/spf13/afero

mem/file.go

@@ -74,14 +74,24 @@ func CreateDir(name string) *FileData {
 }
 
 func ChangeFileName(f *FileData, newname string) {
+	f.Lock()
 	f.name = newname
+	f.Unlock()
 }
 
 func SetMode(f *FileData, mode os.FileMode) {
+	f.Lock()
 	f.mode = mode
+	f.Unlock()
 }
 
 func SetModTime(f *FileData, mtime time.Time) {
+	f.Lock()
+	setModTime(f, mtime)
+	f.Unlock()
+}
+
+func setModTime(f *FileData, mtime time.Time) {
 	f.modtime = mtime
 }
 
@@ -102,7 +112,7 @@ func (f *File) Close() error {
 	f.fileData.Lock()
 	f.closed = true
 	if !f.readOnly {
-		SetModTime(f.fileData, time.Now())
+		setModTime(f.fileData, time.Now())
 	}
 	f.fileData.Unlock()
 	return nil
@@ -197,7 +207,7 @@ func (f *File) Truncate(size int64) error {
 	} else {
 		f.fileData.data = f.fileData.data[0:size]
 	}
-	SetModTime(f.fileData, time.Now())
+	setModTime(f.fileData, time.Now())
 	return nil
 }
 
@@ -236,7 +246,7 @@ func (f *File) Write(b []byte) (n int, err error) {
 		f.fileData.data = append(f.fileData.data[:cur], b...)
 		f.fileData.data = append(f.fileData.data, tail...)
 	}
-	SetModTime(f.fileData, time.Now())
+	setModTime(f.fileData, time.Now())
 
 	atomic.StoreInt64(&f.at, int64(len(f.fileData.data)))
 	return
@@ -261,17 +271,33 @@ type FileInfo struct {
 
 // Implements os.FileInfo
 func (s *FileInfo) Name() string {
+	s.Lock()
 	_, name := filepath.Split(s.name)
+	s.Unlock()
 	return name
 }
-func (s *FileInfo) Mode() os.FileMode  { return s.mode }
-func (s *FileInfo) ModTime() time.Time { return s.modtime }
-func (s *FileInfo) IsDir() bool        { return s.dir }
-func (s *FileInfo) Sys() interface{}   { return nil }
+func (s *FileInfo) Mode() os.FileMode {
+	s.Lock()
+	defer s.Unlock()
+	return s.mode
+}
+func (s *FileInfo) ModTime() time.Time {
+	s.Lock()
+	defer s.Unlock()
+	return s.modtime
+}
+func (s *FileInfo) IsDir() bool {
+	s.Lock()
+	defer s.Unlock()
+	return s.dir
+}
+func (s *FileInfo) Sys() interface{} { return nil }
 func (s *FileInfo) Size() int64 {
 	if s.IsDir() {
 		return int64(42)
 	}
+	s.Lock()
+	defer s.Unlock()
 	return int64(len(s.data))
 }
 

mem/file_test.go

@@ -0,0 +1,154 @@
+package mem
+
+import (
+	"testing"
+	"time"
+)
+
+func Test_FileData_Name_withConcurrence(t *testing.T) {
+	t.Parallel()
+	const someName = "someName"
+	const someOtherName = "someOtherName"
+	d := FileData{
+		name: someName,
+	}
+
+	if d.Name() != someName {
+		t.Errorf("Failed to read correct Name, was %v", d.Name())
+	}
+
+	ChangeFileName(&d, someOtherName)
+	if d.Name() != someOtherName {
+		t.Errorf("Failed to set Name, was %v", d.Name())
+	}
+
+	go func() {
+		ChangeFileName(&d, someName)
+	}()
+
+	if d.Name() != someName && d.Name() != someOtherName {
+		t.Errorf("Failed to read either Name, was %v", d.Name())
+	}
+}
+
+func Test_FileData_ModTime_withConcurrence(t *testing.T) {
+	t.Parallel()
+	someTime := time.Now()
+	someOtherTime := someTime.Add(1 * time.Minute)
+
+	d := FileData{
+		modtime: someTime,
+	}
+
+	s := FileInfo{
+		FileData: &d,
+	}
+
+	if s.ModTime() != someTime {
+		t.Errorf("Failed to read correct value, was %v", s.ModTime())
+	}
+
+	SetModTime(&d, someOtherTime)
+	if s.ModTime() != someOtherTime {
+		t.Errorf("Failed to set ModTime, was %v", s.ModTime())
+	}
+
+	go func() {
+		SetModTime(&d, someTime)
+	}()
+
+	if s.ModTime() != someTime && s.ModTime() != someOtherTime {
+		t.Errorf("Failed to read either modtime, was %v", s.ModTime())
+	}
+}
+
+func Test_FileData_Mode_withConcurrence(t *testing.T) {
+	t.Parallel()
+	const someMode = 0777
+	const someOtherMode = 0660
+
+	d := FileData{
+		mode: someMode,
+	}
+
+	s := FileInfo{
+		FileData: &d,
+	}
+
+	if s.Mode() != someMode {
+		t.Errorf("Failed to read correct value, was %v", s.Mode())
+	}
+
+	SetMode(&d, someOtherMode)
+	if s.Mode() != someOtherMode {
+		t.Errorf("Failed to set Mode, was %v", s.Mode())
+	}
+
+	go func() {
+		SetMode(&d, someMode)
+	}()
+
+	if s.Mode() != someMode && s.Mode() != someOtherMode {
+		t.Errorf("Failed to read either mode, was %v", s.Mode())
+	}
+}
+
+func Test_FileData_IsDir_withConcurrence(t *testing.T) {
+	t.Parallel()
+
+	d := FileData{
+		dir: true,
+	}
+
+	s := FileInfo{
+		FileData: &d,
+	}
+
+	if s.IsDir() != true {
+		t.Errorf("Failed to read correct value, was %v", s.IsDir())
+	}
+
+	go func() {
+		s.Lock()
+		d.dir = false
+		s.Unlock()
+	}()
+
+	//just logging the value to trigger a read:
+	t.Logf("Value is %v", s.IsDir())
+}
+
+func Test_FileData_Size_withConcurrence(t *testing.T) {
+	t.Parallel()
+
+	const someData = "Hello"
+	const someOtherDataSize = "Hello World"
+
+	d := FileData{
+		data: []byte(someData),
+		dir:  false,
+	}
+
+	s := FileInfo{
+		FileData: &d,
+	}
+
+	if s.Size() != int64(len(someData)) {
+		t.Errorf("Failed to read correct value, was %v", s.Size())
+	}
+
+	go func() {
+		s.Lock()
+		d.data = []byte(someOtherDataSize)
+		s.Unlock()
+	}()
+
+	//just logging the value to trigger a read:
+	t.Logf("Value is %v", s.Size())
+
+	//Testing the Dir size case
+	d.dir = true
+	if s.Size() != int64(42) {
+		t.Errorf("Failed to read correct value for dir, was %v", s.Size())
+	}
+}