diff --git a/README.md b/README.md index 255bb36..3110a02 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/TODO.md b/TODO.md index 547648f..7d6a685 100644 --- a/TODO.md +++ b/TODO.md @@ -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, diff --git a/db.go b/db.go index 07b39cf..a3688e2 100644 --- a/db.go +++ b/db.go @@ -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) diff --git a/db_test.go b/db_test.go index 6920d6e..e4b8600 100644 --- a/db_test.go +++ b/db_test.go @@ -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. diff --git a/main_test.go b/main_test.go index 7742365..f266c24 100644 --- a/main_test.go +++ b/main_test.go @@ -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) {