Name the scan lock file from the database's resolved path (closes #101)
check / check (push) Waiting to run
check / check (push) Waiting to run
The lock file was the configured database path with .lock appended, so a scan reaching the database through a symlink to it locked a different file and ran alongside a scan holding the lock through the real path. The lock file is now named from the database's path with symlinks resolved. Before the database exists, its directory is resolved and the file name joined to it, so the error names the same lock file either way. testDBPath resolves its temp dir, so lock errors name the expected path where the temp dir is itself reached through a symlink. Model: opus-5-5
This commit is contained in:
@@ -271,9 +271,12 @@ All three subcommands operate on a single SQLite database file:
|
||||
error (exit 1) telling the user to run `scan` first.
|
||||
- Only one `scan` runs against a database at a time. For its whole run, `scan`
|
||||
holds an exclusive `flock(2)` lock on a lock file beside the database, named
|
||||
by appending `.lock` to the database path (`/var/lib/sfdupes/db.sqlite.lock`
|
||||
by default), taken before it walks the filesystem or opens the database. A
|
||||
second `scan` against the same database does not wait: it fails at once with a
|
||||
by appending `.lock` to the database's path with symlinks resolved
|
||||
(`/var/lib/sfdupes/db.sqlite.lock` by default), taken before it walks the
|
||||
filesystem or opens the database. Every path to one database, through a
|
||||
symlink to the file or to a directory above it, names the same lock file.
|
||||
Before the database exists, only its directory's path is resolved. A second
|
||||
`scan` against the same database does not wait: it fails at once with a
|
||||
one-line error naming the lock file and exits 1, without walking anything or
|
||||
opening the database, and the running scan carries on. The lock file is
|
||||
created on first use, open to its owner only, and left in place: a leftover
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- the scan lock file is named from the database's path with symlinks resolved,
|
||||
so a scan through a symlink to the database fails while another scan holds the
|
||||
lock (2026-10-14, https://git.eeqj.de/sneak/sfdupes/issues/101)
|
||||
|
||||
- `scan` records mtime to the nanosecond, as whole seconds in `mtime` plus
|
||||
`mtime_nsec`, and compares it at that resolution, so a same-size rewrite
|
||||
within the same second is re-hashed (2026-10-07,
|
||||
|
||||
@@ -140,11 +140,13 @@ func openDB(path, params string) (*sql.DB, error) {
|
||||
|
||||
// lockScanDatabase takes the lock that keeps a second scan off the
|
||||
// database at path: an exclusive flock(2) on the file beside it named
|
||||
// path with ".lock" appended, created along with the database's parent
|
||||
// directory if missing. A lock held by another scan fails at once
|
||||
// instead of waiting. The lock lasts until the returned file is closed
|
||||
// or the process ends. The file is never deleted: a scan that deleted
|
||||
// it would let the next scan lock a new file while another still holds
|
||||
// by the database's path with symlinks resolved and ".lock" appended,
|
||||
// so every path to one database names the same lock file. The lock
|
||||
// file is created along with the database's parent directory if
|
||||
// missing. A lock held by another scan fails at once instead of
|
||||
// waiting. The lock lasts until the returned file is closed or the
|
||||
// process ends. The file is never deleted: a scan that deleted it
|
||||
// would let the next scan lock a new file while another still holds
|
||||
// the old one.
|
||||
func lockScanDatabase(path string) (*os.File, error) {
|
||||
err := os.MkdirAll(filepath.Dir(path), dbDirPerm)
|
||||
@@ -152,7 +154,20 @@ func lockScanDatabase(path string) (*os.File, error) {
|
||||
return nil, fmt.Errorf("create database directory: %w", err)
|
||||
}
|
||||
|
||||
lockPath := path + ".lock"
|
||||
resolved, err := filepath.EvalSymlinks(path)
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
// No database yet: resolve its directory, which exists now.
|
||||
var dir string
|
||||
|
||||
dir, err = filepath.EvalSymlinks(filepath.Dir(path))
|
||||
resolved = filepath.Join(dir, filepath.Base(path))
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("resolve database path: %w", err)
|
||||
}
|
||||
|
||||
lockPath := resolved + ".lock"
|
||||
|
||||
//nolint:gosec // the operator chooses the database path
|
||||
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)
|
||||
|
||||
+10
-2
@@ -13,11 +13,19 @@ import (
|
||||
"time"
|
||||
)
|
||||
|
||||
// testDBPath returns a database path inside a fresh temp dir.
|
||||
// testDBPath returns a database path inside a fresh temp dir, with
|
||||
// symlinks resolved as the scan lock resolves them, so the lock file
|
||||
// is this path plus ".lock" even where the temp dir is reached through
|
||||
// a symlink.
|
||||
func testDBPath(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
return filepath.Join(t.TempDir(), "db.sqlite")
|
||||
dir, err := filepath.EvalSymlinks(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
return filepath.Join(dir, "db.sqlite")
|
||||
}
|
||||
|
||||
// openTestDB creates a fresh scan database in a temp dir.
|
||||
|
||||
@@ -854,6 +854,64 @@ func TestRunSecondScanFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// assertSecondScanFails holds the lock on the database at path and
|
||||
// runs a scan against the database SFDUPES_DATABASE names, another
|
||||
// path to the same database. It fails the test unless that scan fails
|
||||
// at once, naming the lock file beside path.
|
||||
func assertSecondScanFails(t *testing.T, path string) {
|
||||
t.Helper()
|
||||
|
||||
holdScanLock(t, path)
|
||||
|
||||
var stderr bytes.Buffer
|
||||
|
||||
code := run([]string{cmdScan, t.TempDir()}, io.Discard, &stderr)
|
||||
if code != exitFatal {
|
||||
t.Errorf("run(scan) = %d, want %d", code, exitFatal)
|
||||
}
|
||||
|
||||
want := "sfdupes: another scan is running (lock held on " +
|
||||
path + ".lock)\n"
|
||||
if got := stderr.String(); got != want {
|
||||
t.Errorf("stderr = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunSecondScanThroughSymlinkFails(t *testing.T) {
|
||||
// README §Database: every path to one database names the same lock
|
||||
// file, so while a scan holds it through the database's real path, a
|
||||
// scan through a symlink fails, naming the real path's lock file.
|
||||
t.Run("symlink to the database", func(t *testing.T) {
|
||||
path := testDBPath(t)
|
||||
t.Setenv(databaseEnv, path)
|
||||
|
||||
scanFixture(t)
|
||||
|
||||
link := filepath.Join(t.TempDir(), "link.sqlite")
|
||||
|
||||
err := os.Symlink(path, link)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
t.Setenv(databaseEnv, link)
|
||||
assertSecondScanFails(t, path)
|
||||
})
|
||||
|
||||
t.Run("symlink to its directory, no database yet", func(t *testing.T) {
|
||||
path := testDBPath(t)
|
||||
link := filepath.Join(t.TempDir(), "link")
|
||||
|
||||
err := os.Symlink(filepath.Dir(path), link)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
t.Setenv(databaseEnv, filepath.Join(link, filepath.Base(path)))
|
||||
assertSecondScanFails(t, path)
|
||||
})
|
||||
}
|
||||
|
||||
func TestRunScanReleasesLock(t *testing.T) {
|
||||
// README §Database: a scan releases the lock however it ends.
|
||||
t.Run("success", func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user