vfs: enforce mutual exclusion in MemFS.Lock

Previously MemFS's Lock method did not implement mutual exclusion under the
assumption that databases are given separate processes which do not share
memory. This commit adapts Lock to enforce mutual exclusion, preventing
accidentally using same FS concurrently from more than one Pebble instance.
It's believed that cockroachdb/cockroach#110645 is the result of Cockroach
acceptance tests opening a MemFS store twice, with one Pebble instance deleting
files from beneath another Pebble instance.

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()
+}

vfs/mem_fs_test.go

@@ -5,13 +5,15 @@
 package vfs
 
 import (
+	"fmt"
 	"io"
 	"os"
 	"sort"
 	"strconv"
 	"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=<filesystem-name> handle=<handle> path=<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=<filesystem-name> path=<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=<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)
+		}
+	})
+}

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