Skip to content

Commit a800a9d

Browse files
committed
Fix concurrency issue in MemMapFs.Mkdir/MkdirAll
* The backing map is protected by a RWMutex * This commit double checks for the existence of the directory inside the write lock to avoid potential data races when multiple goroutines tries to create the same directory. Fixes #361 Fixes #298
1 parent 2a70f2b commit a800a9d

File tree

2 files changed

+89
-0
lines changed

2 files changed

+89
-0
lines changed

memmap.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
142142
}
143143

144144
m.mu.Lock()
145+
// Dobule check that it doesn't exist.
146+
if _, ok := m.getData()[name]; ok {
147+
m.mu.Unlock()
148+
return &os.PathError{Op: "mkdir", Path: name, Err: ErrFileExists}
149+
}
145150
item := mem.CreateDir(name)
146151
mem.SetMode(item, os.ModeDir|perm)
147152
m.getData()[name] = item

memmap_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ package afero
33
import (
44
"fmt"
55
"io"
6+
"io/fs"
67
"os"
78
"path/filepath"
89
"runtime"
10+
"strings"
11+
"sync"
912
"testing"
1013
"time"
1114
)
@@ -692,3 +695,84 @@ func TestMemFsLstatIfPossible(t *testing.T) {
692695
t.Fatalf("Function indicated lstat was called. This should never be true.")
693696
}
694697
}
698+
699+
func TestMemMapFsConfurrentMkdir(t *testing.T) {
700+
const dir = "test_dir"
701+
const n = 1000
702+
mfs := NewMemMapFs().(*MemMapFs)
703+
704+
allFilePaths := make([]string, 0, n)
705+
706+
// run concurrency test
707+
var wg sync.WaitGroup
708+
for i := 0; i < n; i++ {
709+
fp := filepath.Join(
710+
dir,
711+
fmt.Sprintf("%02d", n%10),
712+
fmt.Sprintf("%d.txt", i),
713+
)
714+
allFilePaths = append(allFilePaths, fp)
715+
716+
wg.Add(1)
717+
go func() {
718+
defer wg.Done()
719+
720+
if err := mfs.MkdirAll(filepath.Dir(fp), 0755); err != nil {
721+
t.Error(err)
722+
}
723+
724+
wt, err := mfs.Create(fp)
725+
if err != nil {
726+
t.Error(err)
727+
}
728+
defer func() {
729+
if err := wt.Close(); err != nil {
730+
t.Error(err)
731+
}
732+
}()
733+
734+
// write 30 bytes
735+
for j := 0; j < 10; j++ {
736+
_, err := wt.Write([]byte("000"))
737+
if err != nil {
738+
t.Error(err)
739+
}
740+
}
741+
}()
742+
}
743+
wg.Wait()
744+
745+
// Test1: find all files by full path access
746+
for _, fp := range allFilePaths {
747+
info, err := mfs.Stat(fp)
748+
if err != nil {
749+
t.Error(err)
750+
}
751+
752+
if info.Size() != 30 {
753+
t.Errorf("file size should be 30, but got %d", info.Size())
754+
}
755+
756+
}
757+
758+
// Test2: find all files by walk
759+
foundFiles := make([]string, 0, n)
760+
wErr := Walk(mfs, dir, func(path string, info fs.FileInfo, err error) error {
761+
if err != nil {
762+
t.Error(err)
763+
}
764+
if info.IsDir() {
765+
return nil // skip dir
766+
}
767+
if strings.HasSuffix(info.Name(), ".txt") {
768+
foundFiles = append(foundFiles, path)
769+
}
770+
return nil
771+
})
772+
if wErr != nil {
773+
t.Error(wErr)
774+
}
775+
if len(foundFiles) != n {
776+
t.Errorf("found %d files, but expect %d", len(foundFiles), n)
777+
}
778+
}

0 commit comments

Comments
 (0)