diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 5345ff9d6d..a76b5e329e 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -23,6 +23,7 @@ import ( "github.com/containers/storage/drivers/overlayutils" "github.com/containers/storage/drivers/quota" "github.com/containers/storage/internal/dedup" + "github.com/containers/storage/internal/staging_lockfile" "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/chrootarchive" "github.com/containers/storage/pkg/directory" @@ -30,7 +31,6 @@ import ( "github.com/containers/storage/pkg/fsutils" "github.com/containers/storage/pkg/idmap" "github.com/containers/storage/pkg/idtools" - "github.com/containers/storage/pkg/lockfile" "github.com/containers/storage/pkg/mount" "github.com/containers/storage/pkg/parsers" "github.com/containers/storage/pkg/system" @@ -133,7 +133,7 @@ type Driver struct { stagingDirsLocksMutex sync.Mutex // stagingDirsLocks access is not thread safe, it is required that callers take // stagingDirsLocksMutex on each access to guard against concurrent map writes. - stagingDirsLocks map[string]*lockfile.LockFile + stagingDirsLocks map[string]*staging_lockfile.StagingLockFile supportsIDMappedMounts *bool } @@ -442,7 +442,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) usingComposefs: opts.useComposefs, options: *opts, stagingDirsLocksMutex: sync.Mutex{}, - stagingDirsLocks: make(map[string]*lockfile.LockFile), + stagingDirsLocks: make(map[string]*staging_lockfile.StagingLockFile), } d.naiveDiff = graphdriver.NewNaiveDiffDriver(d, graphdriver.NewNaiveLayerIDMapUpdater(d)) @@ -874,7 +874,9 @@ func (d *Driver) Cleanup() error { func (d *Driver) pruneStagingDirectories() bool { d.stagingDirsLocksMutex.Lock() for _, lock := range d.stagingDirsLocks { - lock.Unlock() + if err := lock.UnlockAndDelete(); err != nil { + logrus.Warnf("Failed to unlock and delete staging lock file: %v", err) + } } clear(d.stagingDirsLocks) d.stagingDirsLocksMutex.Unlock() @@ -886,17 +888,15 @@ func (d *Driver) pruneStagingDirectories() bool { if err == nil { for _, dir := range dirs { stagingDirToRemove := filepath.Join(stagingDirBase, dir.Name()) - lock, err := lockfile.GetLockFile(filepath.Join(stagingDirToRemove, stagingLockFile)) + lock, err := staging_lockfile.TryLockPath(filepath.Join(stagingDirToRemove, stagingLockFile)) if err != nil { anyPresent = true continue } - if err := lock.TryLock(); err != nil { - anyPresent = true - continue - } _ = os.RemoveAll(stagingDirToRemove) - lock.Unlock() + if err := lock.UnlockAndDelete(); err != nil { + logrus.Warnf("Failed to unlock and delete staging lock file: %v", err) + } } } return anyPresent @@ -2178,7 +2178,10 @@ func (d *Driver) CleanupStagingDirectory(stagingDirectory string) error { d.stagingDirsLocksMutex.Lock() if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok { delete(d.stagingDirsLocks, parentStagingDir) - lock.Unlock() + if err := lock.UnlockAndDelete(); err != nil { + d.stagingDirsLocksMutex.Unlock() + return err + } } d.stagingDirsLocksMutex.Unlock() @@ -2233,7 +2236,7 @@ func (d *Driver) ApplyDiffWithDiffer(options *graphdriver.ApplyDiffWithDifferOpt return graphdriver.DriverWithDifferOutput{}, err } - lock, err := lockfile.GetLockFile(filepath.Join(layerDir, stagingLockFile)) + lock, err := staging_lockfile.TryLockPath(filepath.Join(layerDir, stagingLockFile)) if err != nil { return graphdriver.DriverWithDifferOutput{}, err } @@ -2242,13 +2245,14 @@ func (d *Driver) ApplyDiffWithDiffer(options *graphdriver.ApplyDiffWithDifferOpt d.stagingDirsLocksMutex.Lock() delete(d.stagingDirsLocks, layerDir) d.stagingDirsLocksMutex.Unlock() - lock.Unlock() + if err := lock.UnlockAndDelete(); err != nil { + errRet = errors.Join(errRet, err) + } } }() d.stagingDirsLocksMutex.Lock() d.stagingDirsLocks[layerDir] = lock d.stagingDirsLocksMutex.Unlock() - lock.Lock() logrus.Debugf("Applying differ in %s", applyDir) @@ -2274,7 +2278,7 @@ func (d *Driver) ApplyDiffWithDiffer(options *graphdriver.ApplyDiffWithDifferOpt } // ApplyDiffFromStagingDirectory applies the changes using the specified staging directory. -func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *graphdriver.DriverWithDifferOutput, options *graphdriver.ApplyDiffWithDifferOpts) error { +func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *graphdriver.DriverWithDifferOutput, options *graphdriver.ApplyDiffWithDifferOpts) (errRet error) { stagingDirectory := diffOutput.Target parentStagingDir := filepath.Dir(stagingDirectory) @@ -2282,7 +2286,9 @@ func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *gr d.stagingDirsLocksMutex.Lock() if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok { delete(d.stagingDirsLocks, parentStagingDir) - lock.Unlock() + if err := lock.UnlockAndDelete(); err != nil { + errRet = errors.Join(errRet, err) + } } d.stagingDirsLocksMutex.Unlock() }() diff --git a/internal/rawfilelock/rawfilelock.go b/internal/rawfilelock/rawfilelock.go new file mode 100644 index 0000000000..4f340ae3c1 --- /dev/null +++ b/internal/rawfilelock/rawfilelock.go @@ -0,0 +1,64 @@ +package rawfilelock + +import ( + "os" +) + +type LockType byte + +const ( + ReadLock LockType = iota + WriteLock +) + +type FileHandle = fileHandle + +// OpenLock opens a file for locking +// WARNING: This is the underlying file locking primitive of the OS; +// because closing FileHandle releases the lock, it is not suitable for use +// if there is any chance of two concurrent goroutines attempting to use the same lock. +// Most users should use the higher-level operations from internal/staging_lockfile or pkg/lockfile. +func OpenLock(path string, readOnly bool) (FileHandle, error) { + flags := os.O_CREATE + if readOnly { + flags |= os.O_RDONLY + } else { + flags |= os.O_RDWR + } + + fd, err := openHandle(path, flags) + if err == nil { + return fd, nil + } + + return fd, &os.PathError{Op: "open", Path: path, Err: err} +} + +// TryLockFile attempts to lock a file handle +func TryLockFile(fd FileHandle, lockType LockType) error { + return lockHandle(fd, lockType, true) +} + +// LockFile locks a file handle +func LockFile(fd FileHandle, lockType LockType) error { + return lockHandle(fd, lockType, false) +} + +// UnlockAndClose unlocks and closes a file handle +func UnlockAndCloseHandle(fd FileHandle) { + unlockAndCloseHandle(fd) +} + +// CloseHandle closes a file handle without unlocking +// +// WARNING: This is a last-resort function for error handling only! +// On Unix systems, closing a file descriptor automatically releases any locks, +// so "closing without unlocking" is impossible. This function will release +// the lock as a side effect of closing the file. +// +// This function should only be used in error paths where the lock state +// is already corrupted or when giving up on lock management entirely. +// Normal code should use UnlockAndCloseHandle instead. +func CloseHandle(fd FileHandle) { + closeHandle(fd) +} diff --git a/internal/rawfilelock/rawfilelock_test.go b/internal/rawfilelock/rawfilelock_test.go new file mode 100644 index 0000000000..83354069ee --- /dev/null +++ b/internal/rawfilelock/rawfilelock_test.go @@ -0,0 +1,86 @@ +package rawfilelock + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestOpenLock(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + prepare func() (path string, readOnly bool) + }{ + { + name: "file exists (read/write)", + prepare: func() (string, bool) { + tempFile, err := os.CreateTemp(t.TempDir(), "lock-") + require.NoError(t, err) + tempFile.Close() + return tempFile.Name(), false + }, + }, + { + name: "file exists readonly (readonly)", + prepare: func() (string, bool) { + tempFile, err := os.CreateTemp(t.TempDir(), "lock-") + require.NoError(t, err) + tempFile.Close() + return tempFile.Name(), true + }, + }, + { + name: "base dir exists (read/write)", + prepare: func() (string, bool) { + tempDir := os.TempDir() + require.DirExists(t, tempDir) + return filepath.Join(tempDir, "test-1.lock"), false + }, + }, + } { + path, readOnly := tc.prepare() + + fd, err := OpenLock(path, readOnly) + require.NoError(t, err, tc.name) + UnlockAndCloseHandle(fd) + + fd, err = OpenLock(path, readOnly) + require.NoError(t, err) + UnlockAndCloseHandle(fd) + + require.Nil(t, os.RemoveAll(path)) + } +} + +func TestOpenLockNotCreateParentDir(t *testing.T) { + tmpDir := t.TempDir() + lockPath := filepath.Join(tmpDir, "lockfile") + fd, err := OpenLock(lockPath, false) + require.NoError(t, err) + UnlockAndCloseHandle(fd) + + lockPath = filepath.Join(tmpDir, "subdir", "lockfile") + _, err = OpenLock(lockPath, false) + require.Error(t, err) +} + +func TestTryLockFileAndLockFile(t *testing.T) { + tmpFile, err := os.CreateTemp(t.TempDir(), "lockfile") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + fd, err := OpenLock(tmpFile.Name(), false) + require.NoError(t, err) + + require.NoError(t, TryLockFile(fd, WriteLock)) + UnlockAndCloseHandle(fd) + + fd2, err := OpenLock(tmpFile.Name(), false) + require.NoError(t, err) + + require.NoError(t, LockFile(fd2, WriteLock)) + UnlockAndCloseHandle(fd2) +} diff --git a/internal/rawfilelock/rawfilelock_unix.go b/internal/rawfilelock/rawfilelock_unix.go new file mode 100644 index 0000000000..2685540769 --- /dev/null +++ b/internal/rawfilelock/rawfilelock_unix.go @@ -0,0 +1,49 @@ +//go:build !windows + +package rawfilelock + +import ( + "time" + + "golang.org/x/sys/unix" +) + +type fileHandle uintptr + +func openHandle(path string, mode int) (fileHandle, error) { + mode |= unix.O_CLOEXEC + fd, err := unix.Open(path, mode, 0o644) + return fileHandle(fd), err +} + +func lockHandle(fd fileHandle, lType LockType, nonblocking bool) error { + fType := unix.F_RDLCK + if lType != ReadLock { + fType = unix.F_WRLCK + } + lk := unix.Flock_t{ + Type: int16(fType), + Whence: int16(unix.SEEK_SET), + Start: 0, + Len: 0, + } + cmd := unix.F_SETLKW + if nonblocking { + cmd = unix.F_SETLK + } + for { + err := unix.FcntlFlock(uintptr(fd), cmd, &lk) + if err == nil || nonblocking { + return err + } + time.Sleep(10 * time.Millisecond) + } +} + +func unlockAndCloseHandle(fd fileHandle) { + unix.Close(int(fd)) +} + +func closeHandle(fd fileHandle) { + unix.Close(int(fd)) +} diff --git a/internal/rawfilelock/rawfilelock_windows.go b/internal/rawfilelock/rawfilelock_windows.go new file mode 100644 index 0000000000..9c0d692f8a --- /dev/null +++ b/internal/rawfilelock/rawfilelock_windows.go @@ -0,0 +1,48 @@ +//go:build windows + +package rawfilelock + +import ( + "golang.org/x/sys/windows" +) + +const ( + reserved = 0 + allBytes = ^uint32(0) +) + +type fileHandle windows.Handle + +func openHandle(path string, mode int) (fileHandle, error) { + mode |= windows.O_CLOEXEC + fd, err := windows.Open(path, mode, windows.S_IWRITE) + return fileHandle(fd), err +} + +func lockHandle(fd fileHandle, lType LockType, nonblocking bool) error { + flags := 0 + if lType != ReadLock { + flags = windows.LOCKFILE_EXCLUSIVE_LOCK + } + if nonblocking { + flags |= windows.LOCKFILE_FAIL_IMMEDIATELY + } + ol := new(windows.Overlapped) + if err := windows.LockFileEx(windows.Handle(fd), uint32(flags), reserved, allBytes, allBytes, ol); err != nil { + if nonblocking { + return err + } + panic(err) + } + return nil +} + +func unlockAndCloseHandle(fd fileHandle) { + ol := new(windows.Overlapped) + windows.UnlockFileEx(windows.Handle(fd), reserved, allBytes, allBytes, ol) + closeHandle(fd) +} + +func closeHandle(fd fileHandle) { + windows.Close(windows.Handle(fd)) +} diff --git a/internal/staging_lockfile/staging_lockfile.go b/internal/staging_lockfile/staging_lockfile.go new file mode 100644 index 0000000000..1cb2a3327b --- /dev/null +++ b/internal/staging_lockfile/staging_lockfile.go @@ -0,0 +1,147 @@ +package staging_lockfile + +import ( + "fmt" + "os" + "path/filepath" + "sync" + + "github.com/containers/storage/internal/rawfilelock" +) + +// StagingLockFile represents a file lock used to coordinate access to staging areas. +// Typical usage is via CreateAndLock or TryLockPath, both of which return a StagingLockFile +// that must eventually be released with UnlockAndDelete. This ensures that access +// to the staging file is properly synchronized both within and across processes. +// +// WARNING: This struct MUST NOT be created manually. Use the provided helper functions instead. +type StagingLockFile struct { + // Locking invariant: If stagingLockFileLock is not locked, a StagingLockFile for a particular + // path exists if the current process currently owns the lock for that file, and it is recorded in stagingLockFiles. + // + // The following fields can only be accessed by the goroutine owning the lock. + // + // An empty string in the file field means that the lock has been released and the StagingLockFile is no longer valid. + file string // Also the key in stagingLockFiles + fd rawfilelock.FileHandle +} + +const maxRetries = 1000 + +var ( + stagingLockFiles map[string]*StagingLockFile + stagingLockFileLock sync.Mutex +) + +// tryAcquireLockForFile attempts to acquire a lock for the specified file path. +func tryAcquireLockForFile(path string) (*StagingLockFile, error) { + cleanPath, err := filepath.Abs(path) + if err != nil { + return nil, fmt.Errorf("ensuring that path %q is an absolute path: %w", path, err) + } + + stagingLockFileLock.Lock() + defer stagingLockFileLock.Unlock() + + if stagingLockFiles == nil { + stagingLockFiles = make(map[string]*StagingLockFile) + } + + if _, ok := stagingLockFiles[cleanPath]; ok { + return nil, fmt.Errorf("lock %q is used already with other thread", cleanPath) + } + + fd, err := rawfilelock.OpenLock(cleanPath, false) + if err != nil { + return nil, err + } + + if err = rawfilelock.TryLockFile(fd, rawfilelock.WriteLock); err != nil { + // Lock acquisition failed, but holding stagingLockFileLock ensures + // no other goroutine in this process could have obtained a lock for this file, + // so closing it is still safe. + rawfilelock.CloseHandle(fd) + return nil, fmt.Errorf("failed to acquire lock on %q: %w", cleanPath, err) + } + + lockFile := &StagingLockFile{ + file: cleanPath, + fd: fd, + } + + stagingLockFiles[cleanPath] = lockFile + return lockFile, nil +} + +// UnlockAndDelete releases the lock, removes the associated file from the filesystem. +// +// WARNING: After this operation, the StagingLockFile becomes invalid for further use. +func (l *StagingLockFile) UnlockAndDelete() error { + stagingLockFileLock.Lock() + defer stagingLockFileLock.Unlock() + + if l.file == "" { + // Panic when unlocking an unlocked lock. That's a violation + // of the lock semantics and will reveal such. + panic("calling Unlock on unlocked lock") + } + + defer func() { + // It’s important that this happens while we are still holding stagingLockFileLock, to ensure + // that no other goroutine has l.file open = that this close is not unlocking the lock under any + // other goroutine. (defer ordering is LIFO, so this will happen before we release the stagingLockFileLock) + rawfilelock.UnlockAndCloseHandle(l.fd) + delete(stagingLockFiles, l.file) + l.file = "" + }() + if err := os.Remove(l.file); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + +// CreateAndLock creates a new temporary file in the specified directory with the given pattern, +// then creates and locks a StagingLockFile for it. The file is created using os.CreateTemp. +// Typically, the caller would use the returned lock file path to derive a path to the lock-controlled resource +// (e.g. by replacing the "pattern" part of the returned file name with a different prefix) +// Caller MUST call UnlockAndDelete() on the returned StagingLockFile to release the lock and delete the file. +// +// Returns: +// - The locked StagingLockFile +// - The name of created lock file +// - Any error that occurred during the process +// +// If the file cannot be locked, this function will retry up to maxRetries times before failing. +func CreateAndLock(dir string, pattern string) (*StagingLockFile, string, error) { + for try := 0; ; try++ { + file, err := os.CreateTemp(dir, pattern) + if err != nil { + return nil, "", err + } + file.Close() + + path := file.Name() + l, err := tryAcquireLockForFile(path) + if err != nil { + if try < maxRetries { + continue // Retry if the lock cannot be acquired + } + return nil, "", fmt.Errorf( + "failed to allocate lock in %q after %d attempts; last failure on %q: %w", + dir, try, filepath.Base(path), err, + ) + } + + return l, filepath.Base(path), nil + } +} + +// TryLockPath attempts to acquire a lock on an specific path. If the file does not exist, +// it will be created. +// +// Warning: If acquiring a lock is successful, it returns a new StagingLockFile +// instance for the file. Caller MUST call UnlockAndDelete() on the returned StagingLockFile +// to release the lock and delete the file. +func TryLockPath(path string) (*StagingLockFile, error) { + return tryAcquireLockForFile(path) +} diff --git a/internal/staging_lockfile/staging_lockfile_test.go b/internal/staging_lockfile/staging_lockfile_test.go new file mode 100644 index 0000000000..3f8d6d5836 --- /dev/null +++ b/internal/staging_lockfile/staging_lockfile_test.go @@ -0,0 +1,168 @@ +package staging_lockfile + +import ( + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/containers/storage/pkg/reexec" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMain(m *testing.M) { + if reexec.Init() { + return + } + os.Exit(m.Run()) +} + +// subTryLockPath starts a child process. +// The caller must call Wait() on the returned cmd. +func subTryLockPath(path string) (*exec.Cmd, io.ReadCloser, error) { + cmd := reexec.Command("subTryLockPath", path) + rc, err := cmd.StdoutPipe() + if err != nil { + return nil, nil, err + } + if err := cmd.Start(); err != nil { + return nil, nil, err + } + return cmd, rc, nil +} + +// subTryLockPathMain is a child process which tries to opens the StagingLockfile, +// If it has acquired the lock, it will unlock and delete the file. +// Otherwise, it will print an error message to stdout. +func subTryLockPathMain() { + if len(os.Args) != 2 { + fmt.Printf("expected two args, got %d", len(os.Args)) + os.Exit(1) + } + tf, err := TryLockPath(os.Args[1]) + if err != nil { + fmt.Printf("error opening lock file %q: %v", os.Args[1], err) + os.Exit(1) + } + if err := tf.UnlockAndDelete(); err != nil { + fmt.Printf("error unlocking and deleting lock file %q: %v", os.Args[1], err) + os.Exit(1) + } +} + +func init() { + reexec.Register("subTryLockPath", subTryLockPathMain) +} + +func TestCreateAndLock(t *testing.T) { + l, _, err := CreateAndLock(t.TempDir(), "staging-lockfile") + require.NoError(t, err) + + require.NoError(t, l.UnlockAndDelete()) + + require.Empty(t, l.file) + require.Len(t, stagingLockFiles, 0) +} + +func TestTryLockPath(t *testing.T) { + lockFilePath := filepath.Join(t.TempDir(), "test-staging-lockfile") + l, err := TryLockPath(lockFilePath) + require.NoError(t, err) + + require.NoError(t, l.UnlockAndDelete()) + + require.Len(t, stagingLockFiles, 0) + _, err = os.Stat(lockFilePath) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func TestCreateAndLockAndTryLock(t *testing.T) { + tmpDirPath := t.TempDir() + l, path, err := CreateAndLock(tmpDirPath, "locktest") + require.NoError(t, err) + fullPath := filepath.Join(tmpDirPath, path) + defer os.Remove(fullPath) + + _, err = TryLockPath(fullPath) + require.Error(t, err) + + require.NoError(t, l.UnlockAndDelete()) + require.Len(t, stagingLockFiles, 0) + + l2, err := TryLockPath(fullPath) + require.NoError(t, err) + require.NoError(t, l2.UnlockAndDelete()) + + require.Len(t, stagingLockFiles, 0) +} + +func TestUnlockAndDeleteTwice(t *testing.T) { + tmpDirPath := t.TempDir() + l, path, err := CreateAndLock(tmpDirPath, "panic-unlockdelete") + require.NoError(t, err) + fullPath := filepath.Join(tmpDirPath, path) + defer os.Remove(fullPath) + require.NoError(t, l.UnlockAndDelete()) + assert.Panics(t, func() { _ = l.UnlockAndDelete() }, "UnlockAndDelete should panic if not locked") +} + +func TestLockFileRecreation(t *testing.T) { + tmpDirPath := t.TempDir() + l, path, err := CreateAndLock(tmpDirPath, "recreate-lock") + require.NoError(t, err) + require.NoError(t, l.UnlockAndDelete()) + fullPath := filepath.Join(tmpDirPath, path) + + l2, err := TryLockPath(fullPath) + require.NoError(t, err) + require.NoError(t, l2.UnlockAndDelete()) + + require.Len(t, stagingLockFiles, 0) +} + +func TestConcurrentLocking(t *testing.T) { + const n = 10 + ch := make(chan struct{}, n) + for i := 0; i < n; i++ { + go func() { + l, _, err := CreateAndLock(t.TempDir(), "concurrent-lock") + require.NoError(t, err) + require.NoError(t, l.UnlockAndDelete()) + ch <- struct{}{} + }() + } + for i := 0; i < n; i++ { + <-ch + } + require.Len(t, stagingLockFiles, 0) +} + +func TestTryLockPathMultiProcess(t *testing.T) { + tmpDirPath := t.TempDir() + lockfile, path, err := CreateAndLock(tmpDirPath, "test-staging-lockfile") + require.NoError(t, err) + fullPath := filepath.Join(tmpDirPath, path) + + expectedErrMsg := fmt.Sprintf("error opening lock file %q: failed to acquire lock on ", fullPath) + tryLockTimes := 3 + for i := 0; i < tryLockTimes; i++ { + cmd, stdout, err := subTryLockPath(fullPath) + require.NoError(t, err) + stderrBuf := new(strings.Builder) + _, err = io.Copy(stderrBuf, stdout) + require.NoError(t, err) + require.Error(t, cmd.Wait()) + require.Contains(t, stderrBuf.String(), expectedErrMsg) + } + require.NoError(t, lockfile.UnlockAndDelete()) + + cmd, _, err := subTryLockPath(fullPath) + require.NoError(t, err) + require.NoError(t, cmd.Wait()) + + require.Len(t, stagingLockFiles, 0) +} diff --git a/pkg/lockfile/lockfile.go b/pkg/lockfile/lockfile.go index 52f6c0a62c..dfe81c2458 100644 --- a/pkg/lockfile/lockfile.go +++ b/pkg/lockfile/lockfile.go @@ -6,6 +6,8 @@ import ( "path/filepath" "sync" "time" + + "github.com/containers/storage/internal/rawfilelock" ) // A Locker represents a file lock where the file is used to cache an @@ -55,13 +57,6 @@ type Locker interface { AssertLockedForWriting() } -type lockType byte - -const ( - readLock lockType = iota - writeLock -) - // LockFile represents a file lock where the file is used to cache an // identifier of the last party that made changes to whatever's being protected // by the lock. @@ -79,12 +74,12 @@ type LockFile struct { stateMutex *sync.Mutex counter int64 lw LastWrite // A global value valid as of the last .Touch() or .Modified() - lockType lockType + lockType rawfilelock.LockType locked bool // The following fields are only modified on transitions between counter == 0 / counter != 0. // Thus, they can be safely accessed by users _that currently hold the LockFile_ without locking. // In other cases, they need to be protected using stateMutex. - fd fileHandle + fd rawfilelock.FileHandle } var ( @@ -129,12 +124,12 @@ func (l *LockFile) Lock() { if l.ro { panic("can't take write lock on read-only lock file") } - l.lock(writeLock) + l.lock(rawfilelock.WriteLock) } // RLock locks the lockfile as a reader. func (l *LockFile) RLock() { - l.lock(readLock) + l.lock(rawfilelock.ReadLock) } // TryLock attempts to lock the lockfile as a writer. Panic if the lock is a read-only one. @@ -142,12 +137,12 @@ func (l *LockFile) TryLock() error { if l.ro { panic("can't take write lock on read-only lock file") } - return l.tryLock(writeLock) + return l.tryLock(rawfilelock.WriteLock) } // TryRLock attempts to lock the lockfile as a reader. func (l *LockFile) TryRLock() error { - return l.tryLock(readLock) + return l.tryLock(rawfilelock.ReadLock) } // Unlock unlocks the lockfile. @@ -172,9 +167,9 @@ func (l *LockFile) Unlock() { l.locked = false // Close the file descriptor on the last unlock, releasing the // file lock. - unlockAndCloseHandle(l.fd) + rawfilelock.UnlockAndCloseHandle(l.fd) } - if l.lockType == readLock { + if l.lockType == rawfilelock.ReadLock { l.rwMutex.RUnlock() } else { l.rwMutex.Unlock() @@ -206,7 +201,7 @@ func (l *LockFile) AssertLockedForWriting() { l.AssertLocked() // Like AssertLocked, don’t even bother with l.stateMutex. - if l.lockType == readLock { + if l.lockType == rawfilelock.ReadLock { panic("internal error: lock is not held for writing") } } @@ -273,7 +268,7 @@ func (l *LockFile) Touch() error { return err } l.stateMutex.Lock() - if !l.locked || (l.lockType == readLock) { + if !l.locked || (l.lockType == rawfilelock.ReadLock) { panic("attempted to update last-writer in lockfile without the write lock") } defer l.stateMutex.Unlock() @@ -324,6 +319,24 @@ func getLockfile(path string, ro bool) (*LockFile, error) { return lockFile, nil } +// openLock opens a lock file at the specified path, creating the parent directory if it does not exist. +func openLock(path string, readOnly bool) (rawfilelock.FileHandle, error) { + fd, err := rawfilelock.OpenLock(path, readOnly) + if err == nil { + return fd, nil + } + + // the directory of the lockfile seems to be removed, try to create it + if os.IsNotExist(err) { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fd, fmt.Errorf("creating lock file directory: %w", err) + } + + return openLock(path, readOnly) + } + return fd, &os.PathError{Op: "open", Path: path, Err: err} +} + // createLockFileForPath returns new *LockFile object, possibly (depending on the platform) // working inter-process and associated with the specified path. // @@ -343,11 +356,11 @@ func createLockFileForPath(path string, ro bool) (*LockFile, error) { if err != nil { return nil, err } - unlockAndCloseHandle(fd) + rawfilelock.UnlockAndCloseHandle(fd) - lType := writeLock + lType := rawfilelock.WriteLock if ro { - lType = readLock + lType = rawfilelock.ReadLock } return &LockFile{ @@ -362,40 +375,10 @@ func createLockFileForPath(path string, ro bool) (*LockFile, error) { }, nil } -// openLock opens the file at path and returns the corresponding file -// descriptor. The path is opened either read-only or read-write, -// depending on the value of ro argument. -// -// openLock will create the file and its parent directories, -// if necessary. -func openLock(path string, ro bool) (fd fileHandle, err error) { - flags := os.O_CREATE - if ro { - flags |= os.O_RDONLY - } else { - flags |= os.O_RDWR - } - fd, err = openHandle(path, flags) - if err == nil { - return fd, nil - } - - // the directory of the lockfile seems to be removed, try to create it - if os.IsNotExist(err) { - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - return fd, fmt.Errorf("creating lock file directory: %w", err) - } - - return openLock(path, ro) - } - - return fd, &os.PathError{Op: "open", Path: path, Err: err} -} - // lock locks the lockfile via syscall based on the specified type and // command. -func (l *LockFile) lock(lType lockType) { - if lType == readLock { +func (l *LockFile) lock(lType rawfilelock.LockType) { + if lType == rawfilelock.ReadLock { l.rwMutex.RLock() } else { l.rwMutex.Lock() @@ -413,7 +396,7 @@ func (l *LockFile) lock(lType lockType) { // Optimization: only use the (expensive) syscall when // the counter is 0. In this case, we're either the first // reader lock or a writer lock. - if err := lockHandle(l.fd, lType, false); err != nil { + if err := rawfilelock.LockFile(l.fd, lType); err != nil { panic(err) } } @@ -424,10 +407,10 @@ func (l *LockFile) lock(lType lockType) { // lock locks the lockfile via syscall based on the specified type and // command. -func (l *LockFile) tryLock(lType lockType) error { +func (l *LockFile) tryLock(lType rawfilelock.LockType) error { var success bool var rwMutexUnlocker func() - if lType == readLock { + if lType == rawfilelock.ReadLock { success = l.rwMutex.TryRLock() rwMutexUnlocker = l.rwMutex.RUnlock } else { @@ -451,8 +434,8 @@ func (l *LockFile) tryLock(lType lockType) error { // Optimization: only use the (expensive) syscall when // the counter is 0. In this case, we're either the first // reader lock or a writer lock. - if err = lockHandle(l.fd, lType, true); err != nil { - closeHandle(fd) + if err = rawfilelock.TryLockFile(l.fd, lType); err != nil { + rawfilelock.CloseHandle(fd) rwMutexUnlocker() return err } diff --git a/pkg/lockfile/lockfile_test.go b/pkg/lockfile/lockfile_test.go index d8513575f6..32dd5eb281 100644 --- a/pkg/lockfile/lockfile_test.go +++ b/pkg/lockfile/lockfile_test.go @@ -4,7 +4,6 @@ import ( "io" "os" "os/exec" - "path/filepath" "runtime" "sync" "sync/atomic" @@ -844,59 +843,3 @@ func TestLockfileMultiprocessModifiedSince(t *testing.T) { require.NoError(t, err) assert.False(t, modified) } - -func TestOpenLock(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - name string - prepare func() (path string, readOnly bool) - }{ - { - name: "file exists (read/write)", - prepare: func() (string, bool) { - tempFile, err := os.CreateTemp("", "lock-") - require.NoError(t, err) - tempFile.Close() - return tempFile.Name(), false - }, - }, - { - name: "file exists readonly (readonly)", - prepare: func() (string, bool) { - tempFile, err := os.CreateTemp("", "lock-") - require.NoError(t, err) - tempFile.Close() - return tempFile.Name(), true - }, - }, - { - name: "base dir exists (read/write)", - prepare: func() (string, bool) { - tempDir := os.TempDir() - require.DirExists(t, tempDir) - return filepath.Join(tempDir, "test-1.lock"), false - }, - }, - { - name: "base dir not exists (read/write)", - prepare: func() (string, bool) { - tempDir, err := os.MkdirTemp("", "lock-") - require.NoError(t, err) - return filepath.Join(tempDir, "subdir", "test-1.lock"), false - }, - }, - } { - path, readOnly := tc.prepare() - - fd, err := openLock(path, readOnly) - require.NoError(t, err, tc.name) - unlockAndCloseHandle(fd) - - fd, err = openLock(path, readOnly) - require.NoError(t, err) - unlockAndCloseHandle(fd) - - require.Nil(t, os.RemoveAll(path)) - } -} diff --git a/pkg/lockfile/lockfile_unix.go b/pkg/lockfile/lockfile_unix.go index 885f2f88a2..14c27c51fb 100644 --- a/pkg/lockfile/lockfile_unix.go +++ b/pkg/lockfile/lockfile_unix.go @@ -9,8 +9,6 @@ import ( "golang.org/x/sys/unix" ) -type fileHandle uintptr - // GetLastWrite returns a LastWrite value corresponding to current state of the lock. // This is typically called before (_not after_) loading the state when initializing a consumer // of the data protected by the lock. @@ -66,41 +64,3 @@ func (l *LockFile) TouchedSince(when time.Time) bool { touched := time.Unix(mtim.Unix()) return when.Before(touched) } - -func openHandle(path string, mode int) (fileHandle, error) { - mode |= unix.O_CLOEXEC - fd, err := unix.Open(path, mode, 0o644) - return fileHandle(fd), err -} - -func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error { - fType := unix.F_RDLCK - if lType != readLock { - fType = unix.F_WRLCK - } - lk := unix.Flock_t{ - Type: int16(fType), - Whence: int16(unix.SEEK_SET), - Start: 0, - Len: 0, - } - cmd := unix.F_SETLKW - if nonblocking { - cmd = unix.F_SETLK - } - for { - err := unix.FcntlFlock(uintptr(fd), cmd, &lk) - if err == nil || nonblocking { - return err - } - time.Sleep(10 * time.Millisecond) - } -} - -func unlockAndCloseHandle(fd fileHandle) { - unix.Close(int(fd)) -} - -func closeHandle(fd fileHandle) { - unix.Close(int(fd)) -} diff --git a/pkg/lockfile/lockfile_windows.go b/pkg/lockfile/lockfile_windows.go index 0cc1c50cc8..e66f7bfbbc 100644 --- a/pkg/lockfile/lockfile_windows.go +++ b/pkg/lockfile/lockfile_windows.go @@ -14,8 +14,6 @@ const ( allBytes = ^uint32(0) ) -type fileHandle windows.Handle - // GetLastWrite returns a LastWrite value corresponding to current state of the lock. // This is typically called before (_not after_) loading the state when initializing a consumer // of the data protected by the lock. @@ -73,37 +71,3 @@ func (l *LockFile) TouchedSince(when time.Time) bool { } return when.Before(stat.ModTime()) } - -func openHandle(path string, mode int) (fileHandle, error) { - mode |= windows.O_CLOEXEC - fd, err := windows.Open(path, mode, windows.S_IWRITE) - return fileHandle(fd), err -} - -func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error { - flags := 0 - if lType != readLock { - flags = windows.LOCKFILE_EXCLUSIVE_LOCK - } - if nonblocking { - flags |= windows.LOCKFILE_FAIL_IMMEDIATELY - } - ol := new(windows.Overlapped) - if err := windows.LockFileEx(windows.Handle(fd), uint32(flags), reserved, allBytes, allBytes, ol); err != nil { - if nonblocking { - return err - } - panic(err) - } - return nil -} - -func unlockAndCloseHandle(fd fileHandle) { - ol := new(windows.Overlapped) - windows.UnlockFileEx(windows.Handle(fd), reserved, allBytes, allBytes, ol) - closeHandle(fd) -} - -func closeHandle(fd fileHandle) { - windows.Close(windows.Handle(fd)) -}