diff --git a/vfs/mem_fs.go b/vfs/mem_fs.go index bb9440a0da..ef6c87d7c7 100644 --- a/vfs/mem_fs.go +++ b/vfs/mem_fs.go @@ -14,6 +14,7 @@ import ( "strings" "sync" "sync/atomic" + "syscall" "time" "github.com/cockroachdb/errors" @@ -78,6 +79,11 @@ type MemFS struct { mu sync.Mutex root *memNode + // lockFiles holds a map of open file locks. Presence in this map indicates + // a file lock is currently held. Keys are strings holding the path of the + // locked file. The stored value is untyped and unused; only presence of + // the key within the map is significant. + lockedFiles sync.Map strict bool ignoreSyncs bool // Windows has peculiar semantics with respect to hard links and deleting @@ -457,9 +463,31 @@ func (y *MemFS) MkdirAll(dirname string, perm os.FileMode) error { // Lock implements FS.Lock. func (y *MemFS) Lock(fullname string) (io.Closer, error) { // FS.Lock excludes other processes, but other processes cannot see this - // process' memory. We translate Lock into Create so that have the normal - // detection of non-existent directory paths. - return y.Create(fullname) + // process' memory. However some uses (eg, Cockroach tests) may open and + // close the same MemFS-backed database multiple times. We want mutual + // exclusion in this case too. See cockroachdb/cockroach#110645. + _, loaded := y.lockedFiles.Swap(fullname, nil /* the value itself is insignificant */) + if loaded { + // This file lock has already been acquired. On unix, this results in + // either EACCES or EAGAIN so we mimic. + return nil, syscall.EAGAIN + } + // Otherwise, we successfully acquired the lock. Locks are visible in the + // parent directory listing, and they also must be created under an existent + // directory. Create the path so that we have the normal detection of + // non-existent directory paths, and make the lock visible when listing + // directory entries. + f, err := y.Create(fullname) + if err != nil { + // "Release" the lock since we failed. + y.lockedFiles.Delete(fullname) + return nil, err + } + return &memFileLock{ + y: y, + f: f, + fullname: fullname, + }, nil } // List implements FS.List. @@ -787,3 +815,18 @@ func (f *memFile) Fd() uintptr { func (f *memFile) Flush() error { return nil } + +type memFileLock struct { + y *MemFS + f File + fullname string +} + +func (l *memFileLock) Close() error { + if l.y == nil { + return nil + } + l.y.lockedFiles.Delete(l.fullname) + l.y = nil + return l.f.Close() +} diff --git a/vfs/mem_fs_test.go b/vfs/mem_fs_test.go index 84e23de1bd..592b392a28 100644 --- a/vfs/mem_fs_test.go +++ b/vfs/mem_fs_test.go @@ -5,6 +5,7 @@ package vfs import ( + "fmt" "io" "os" "sort" @@ -12,6 +13,7 @@ import ( "strings" "testing" + "github.com/cockroachdb/datadriven" "github.com/stretchr/testify/require" ) @@ -393,3 +395,66 @@ func TestStrictFS(t *testing.T) { } runTestCases(t, testCases, fs) } + +func TestMemFSLock(t *testing.T) { + filesystems := map[string]FS{} + fileLocks := map[string]io.Closer{} + + datadriven.RunTest(t, "testdata/memfs_lock", func(t *testing.T, td *datadriven.TestData) string { + switch td.Cmd { + case "mkfs": + for _, arg := range td.CmdArgs { + filesystems[arg.String()] = NewMem() + } + return "OK" + + // lock fs= handle= path= + case "lock": + var filesystemName string + var path string + var handle string + td.ScanArgs(t, "fs", &filesystemName) + td.ScanArgs(t, "path", &path) + td.ScanArgs(t, "handle", &handle) + fs := filesystems[filesystemName] + if fs == nil { + return fmt.Sprintf("filesystem %q doesn't exist", filesystemName) + } + l, err := fs.Lock(path) + if err != nil { + return err.Error() + } + fileLocks[handle] = l + return "OK" + + // mkdirall fs= path= + case "mkdirall": + var filesystemName string + var path string + td.ScanArgs(t, "fs", &filesystemName) + td.ScanArgs(t, "path", &path) + fs := filesystems[filesystemName] + if fs == nil { + return fmt.Sprintf("filesystem %q doesn't exist", filesystemName) + } + err := fs.MkdirAll(path, 0755) + if err != nil { + return err.Error() + } + return "OK" + + // close handle= + case "close": + var handle string + td.ScanArgs(t, "handle", &handle) + err := fileLocks[handle].Close() + delete(fileLocks, handle) + if err != nil { + return err.Error() + } + return "OK" + default: + return fmt.Sprintf("unrecognized command %q", td.Cmd) + } + }) +} diff --git a/vfs/testdata/memfs_lock b/vfs/testdata/memfs_lock new file mode 100644 index 0000000000..0ac265580e --- /dev/null +++ b/vfs/testdata/memfs_lock @@ -0,0 +1,55 @@ +mkfs A B +---- +OK + +# +# Locking a path with parents that don't exist should error. +# + +lock fs=A path=a/b/c handle=fsApathABC +---- +open a/b/c: file does not exist + +# +# If we create the parents, it should succeed. +# + +mkdirall fs=A path=a/b +---- +OK + +lock fs=A path=a/b/c handle=fsApathABC +---- +OK + +# +# Locking the same path on the same filesystem should fail with EAGAIN. +# + +lock fs=A path=a/b/c handle=bogus +---- +resource temporarily unavailable + +# +# Locking the same path on a DIFFERENT filesystem should succeed. +# + +mkdirall fs=B path=a/b +---- +OK + +lock fs=B path=a/b/c handle=fsBpathABC +---- +OK + +# +# Releasing the lock on fs A should allow us to reacquire it. +# + +close handle=fsApathABC +---- +OK + +lock fs=A path=a/b/c handle=fsApathABC +---- +OK