Deduplicate code

Signed-off-by: Jan Rodák <[email protected]>

Author: Honny1

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

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

internal/staging_lockfile/staging_lockfile_unix.go renamed to internal/rawfilelock/rawfilelock_unix.go

@@ -1,6 +1,6 @@
 //go:build !windows
 
-package staging_lockfile
+package rawfilelock
 
 import (
 	"time"
@@ -16,9 +16,9 @@ func openHandle(path string, mode int) (fileHandle, error) {
 	return fileHandle(fd), err
 }
 
-func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error {
+func lockHandle(fd fileHandle, lType LockType, nonblocking bool) error {
 	fType := unix.F_RDLCK
-	if lType != readLock {
+	if lType != ReadLock {
 		fType = unix.F_WRLCK
 	}
 	lk := unix.Flock_t{

internal/staging_lockfile/staging_lockfile_windows.go renamed to internal/rawfilelock/rawfilelock_windows.go

@@ -1,6 +1,6 @@
 //go:build windows
 
-package staging_lockfile
+package rawfilelock
 
 import (
 	"golang.org/x/sys/windows"
@@ -19,9 +19,9 @@ func openHandle(path string, mode int) (fileHandle, error) {
 	return fileHandle(fd), err
 }
 
-func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error {
+func lockHandle(fd fileHandle, lType LockType, nonblocking bool) error {
 	flags := 0
-	if lType != readLock {
+	if lType != ReadLock {
 		flags = windows.LOCKFILE_EXCLUSIVE_LOCK
 	}
 	if nonblocking {

internal/staging_lockfile/staging_lockfile.go

@@ -2,16 +2,10 @@ package staging_lockfile
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"sync"
-)
-
-type lockType byte
 
-const (
-	readLock lockType = iota
-	writeLock
+	"github.com/containers/storage/internal/rawfilelock"
 )
 
 // StagingLockFile represents a file lock used to coordinate access to staging areas.
@@ -30,7 +24,7 @@ type StagingLockFile struct {
 	// stateMutex is used to synchronize concurrent accesses to the state below
 	stateMutex *sync.Mutex
 	locked     bool
-	fd         fileHandle
+	fd         rawfilelock.FileHandle
 }
 
 var (
@@ -86,11 +80,11 @@ func getLockfile(path string) (*StagingLockFile, error) {
 // This function will be called at most once for each unique path within a process.
 func createStagingLockFileForPath(path string) (*StagingLockFile, error) {
 	// Check if we can open the lock.
-	fd, err := openLock(path)
+	fd, err := rawfilelock.OpenLock(path, false)
 	if err != nil {
 		return nil, err
 	}
-	unlockAndCloseHandle(fd)
+	rawfilelock.UnlockAndCloseHandle(fd)
 
 	return &StagingLockFile{
 		file:       path,
@@ -100,30 +94,6 @@ func createStagingLockFileForPath(path string) (*StagingLockFile, error) {
 	}, nil
 }
 
-// openLock opens the file at the specified path with read-write access.
-// It creates the file and any necessary parent directories if they don't exist.
-// Returns a file handle that can be used for locking.
-func openLock(path string) (fd fileHandle, err error) {
-	flags := os.O_CREATE
-	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)
-	}
-
-	return fd, &os.PathError{Op: "open", Path: path, Err: err}
-}
-
 // tryLock attempts to acquire an exclusive lock on the StagingLockFile without blocking.
 // It first tries to acquire the internal rwMutex, then opens and tries to lock the file.
 // Returns nil on success or an error if any step fails.
@@ -136,15 +106,15 @@ func (l *StagingLockFile) tryLock() error {
 	}
 	l.stateMutex.Lock()
 	defer l.stateMutex.Unlock()
-	fd, err := openLock(l.file)
+	fd, err := rawfilelock.OpenLock(l.file, false)
 	if err != nil {
 		rwMutexUnlocker()
 		return err
 	}
 	l.fd = fd
 
-	if err = lockHandle(l.fd, writeLock, true); err != nil {
-		closeHandle(fd)
+	if err = rawfilelock.TryLockFile(l.fd, rawfilelock.WriteLock); err != nil {
+		rawfilelock.CloseHandle(fd)
 		rwMutexUnlocker()
 		return err
 	}

internal/staging_lockfile/staging_lockfile_test.go

@@ -4,7 +4,6 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"runtime"
 	"sync"
 	"sync/atomic"
@@ -229,50 +228,3 @@ func TestLockfileMultiProcess(t *testing.T) {
 	wg.Wait()
 	assert.True(t, whighest == 1, "expected to have no more than one writer lock active at a time, had %d", whighest)
 }
-
-func TestOpenLock(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range []struct {
-		name    string
-		prepare func() (path string)
-	}{
-		{
-			name: "file exists (read/write)",
-			prepare: func() string {
-				tempFile, err := os.CreateTemp("", "lock-")
-				require.NoError(t, err)
-				tempFile.Close()
-				return tempFile.Name()
-			},
-		},
-		{
-			name: "base dir exists (read/write)",
-			prepare: func() string {
-				tempDir := os.TempDir()
-				require.DirExists(t, tempDir)
-				return filepath.Join(tempDir, "test-1.lock")
-			},
-		},
-		{
-			name: "base dir not exists (read/write)",
-			prepare: func() string {
-				tempDir, err := os.MkdirTemp("", "lock-")
-				require.NoError(t, err)
-				return filepath.Join(tempDir, "subdir", "test-1.lock")
-			},
-		},
-	} {
-		path := tc.prepare()
-
-		fd, err := openLock(path)
-		require.NoError(t, err, tc.name)
-		unlockAndCloseHandle(fd)
-
-		fd, err = openLock(path)
-		require.NoError(t, err)
-		unlockAndCloseHandle(fd)
-
-		require.Nil(t, os.RemoveAll(path))
-	}
-}