From b7045b15fc6102c86747c8d1dcc26733e7bbe0dc Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Fri, 14 Apr 2023 10:14:28 +0600 Subject: [PATCH 1/3] use mutex for mapfs MkdirAll --- pkg/mapfs/fs.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/mapfs/fs.go b/pkg/mapfs/fs.go index d075b02d233..954d9ff1f0e 100644 --- a/pkg/mapfs/fs.go +++ b/pkg/mapfs/fs.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" "golang.org/x/exp/slices" @@ -27,6 +28,7 @@ var _ allFS = &FS{} // FS is an in-memory filesystem type FS struct { + mu sync.Mutex root *file } @@ -42,6 +44,7 @@ func New() *FS { }, files: syncx.Map[string, *file]{}, }, + mu: sync.Mutex{}, } } @@ -134,6 +137,13 @@ func (m *FS) WriteVirtualFile(path string, data []byte, mode fs.FileMode) error // If path is already a directory, MkdirAll does nothing // and returns nil. func (m *FS) MkdirAll(path string, perm fs.FileMode) error { + // we can overwrite folders when making folders in parallel + // e.g. we have `foo/bar/folder1/file1` and `foo/bar/folder2/file2` + // file1 checks that `foo/bar` folder doesn't exist + // at that moment file2 creates `foo/bar/folder2` folder + // but file1 doesn't know about this and overwrite `foo/bar` folder( and removes `foo/bar/folder2`) + m.mu.Lock() + defer m.mu.Unlock() return m.root.MkdirAll(cleanPath(path), perm) } From 95759da09a37a1da5d2307671d69480cb53b89f1 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Sun, 16 Apr 2023 17:23:32 +0300 Subject: [PATCH 2/3] fix: use LoadOrStore --- pkg/mapfs/file.go | 34 +++++++++++++++++----------------- pkg/mapfs/fs.go | 5 ----- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/mapfs/file.go b/pkg/mapfs/file.go index b19cee04116..9b2d32aa720 100644 --- a/pkg/mapfs/file.go +++ b/pkg/mapfs/file.go @@ -176,24 +176,24 @@ func (f *file) MkdirAll(path string, perm fs.FileMode) error { return nil } - sub, ok := f.files.Load(parts[0]) - if ok && !sub.stat.IsDir() { - return fs.ErrExist - } else if !ok { - if perm&fs.ModeDir == 0 { - perm |= fs.ModeDir - } + if perm&fs.ModeDir == 0 { + perm |= fs.ModeDir + } - sub = &file{ - stat: fileStat{ - name: parts[0], - size: 0x100, - modTime: time.Now(), - mode: perm, - }, - files: syncx.Map[string, *file]{}, - } - f.files.Store(parts[0], sub) + sub := &file{ + stat: fileStat{ + name: parts[0], + size: 0x100, + modTime: time.Now(), + mode: perm, + }, + files: syncx.Map[string, *file]{}, + } + + // Create the directory when the key is not present + sub, loaded := f.files.LoadOrStore(parts[0], sub) + if loaded && !sub.stat.IsDir() { + return fs.ErrExist } if len(parts) == 1 { diff --git a/pkg/mapfs/fs.go b/pkg/mapfs/fs.go index 954d9ff1f0e..ba65519d288 100644 --- a/pkg/mapfs/fs.go +++ b/pkg/mapfs/fs.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "strings" - "sync" "time" "golang.org/x/exp/slices" @@ -28,7 +27,6 @@ var _ allFS = &FS{} // FS is an in-memory filesystem type FS struct { - mu sync.Mutex root *file } @@ -44,7 +42,6 @@ func New() *FS { }, files: syncx.Map[string, *file]{}, }, - mu: sync.Mutex{}, } } @@ -142,8 +139,6 @@ func (m *FS) MkdirAll(path string, perm fs.FileMode) error { // file1 checks that `foo/bar` folder doesn't exist // at that moment file2 creates `foo/bar/folder2` folder // but file1 doesn't know about this and overwrite `foo/bar` folder( and removes `foo/bar/folder2`) - m.mu.Lock() - defer m.mu.Unlock() return m.root.MkdirAll(cleanPath(path), perm) } From 77ba1fa8276994a73cd5708dcc0eacd238b86bda Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Sun, 16 Apr 2023 17:24:11 +0300 Subject: [PATCH 3/3] remove comment --- pkg/mapfs/fs.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/mapfs/fs.go b/pkg/mapfs/fs.go index ba65519d288..d075b02d233 100644 --- a/pkg/mapfs/fs.go +++ b/pkg/mapfs/fs.go @@ -134,11 +134,6 @@ func (m *FS) WriteVirtualFile(path string, data []byte, mode fs.FileMode) error // If path is already a directory, MkdirAll does nothing // and returns nil. func (m *FS) MkdirAll(path string, perm fs.FileMode) error { - // we can overwrite folders when making folders in parallel - // e.g. we have `foo/bar/folder1/file1` and `foo/bar/folder2/file2` - // file1 checks that `foo/bar` folder doesn't exist - // at that moment file2 creates `foo/bar/folder2` folder - // but file1 doesn't know about this and overwrite `foo/bar` folder( and removes `foo/bar/folder2`) return m.root.MkdirAll(cleanPath(path), perm) }