1 Commits
Author SHA1 Message Date
clawbot 6ccb9d7899 Name the scan lock file from the database's resolved path (closes #101)
check / check (push) Canceled after 0s
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
2026-10-14 17:46:38 +00:00
6 changed files with 38 additions and 112 deletions
+10 -12
View File
@@ -274,18 +274,16 @@ All three subcommands operate on a single SQLite database file:
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. Just
before it takes the lock, `scan` creates the database file, empty, if it is
missing (through a symlink at the database path, it creates the symlink's
target), so the lock file is named the same way before the first scan sets up
the database as after. 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 file blocks nothing, because the lock ends with the process
holding it however it ends, a fatal error or an interrupt included, and
deleting the file while a scan runs would let a second scan start. `report`
and `trees` never take the lock, so they run during a scan.
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
file blocks nothing, because the lock ends with the process holding it however
it ends, a fatal error or an interrupt included, and deleting the file while a
scan runs would let a second scan start. `report` and `trees` never take the
lock, so they run during a scan.
- While `scan` runs, the database is in WAL journal mode with a busy timeout, so
running a report while a cron `scan` is in progress is safe. The filesystem is
authoritative; the database is an eventually-consistent reflection of it.
+2 -7
View File
@@ -30,13 +30,8 @@
# Completed Steps
- the scan lock file is named from the database's path with symlinks resolved,
after `scan` creates the database file if it is missing, 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)
- on a terminal, a warning for the last file of a phase no longer erases the
phase's finished bar: the bar is drawn again below the warning (2026-10-14,
https://git.eeqj.de/sneak/sfdupes/issues/102)
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
+14 -21
View File
@@ -35,10 +35,6 @@ const schemaVersion = 1
// scan.
const dbDirPerm = 0o755
// dbFilePerm is the mode for a database file created by scan, the mode
// SQLite gives a database file it creates.
const dbFilePerm = 0o644
// lockFilePerm is the mode for the scan lock file. Anyone who can open
// the file can hold the lock and keep every scan from running, so it
// is open to its owner only.
@@ -145,31 +141,28 @@ 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
// by the database's path with symlinks resolved and ".lock" appended,
// so every path to one database names the same lock file. If missing,
// the database's parent directory and an empty database file are
// created first, so the lock file is named the same way before the
// database is set up as after. 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 lock 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.
// 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)
if err != nil {
return nil, fmt.Errorf("create database directory: %w", err)
}
// Like SQLite, this follows a symlink at path and creates its
// target. SQLite opens an empty file as a new database.
//nolint:gosec // the operator chooses the database path
dbFile, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, dbFilePerm)
if err != nil {
return nil, fmt.Errorf("create database: %w", err)
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))
}
_ = dbFile.Close()
resolved, err := filepath.EvalSymlinks(path)
if err != nil {
return nil, fmt.Errorf("resolve database path: %w", err)
}
+12 -43
View File
@@ -822,7 +822,7 @@ func holdScanLock(t *testing.T, path string) {
func TestRunSecondScanFails(t *testing.T) {
// README §Database: while one scan holds the lock, a second scan
// fails at once, naming the lock file, without opening the
// fails at once, naming the lock file, without creating the
// database.
path := testDBPath(t)
t.Setenv(databaseEnv, path)
@@ -848,25 +848,21 @@ func TestRunSecondScanFails(t *testing.T) {
t.Errorf("stdout = %q, want nothing (data only)", got)
}
// Taking the lock created the database file, empty; opening it as
// a database would have written to it.
info, err := os.Stat(path)
if err != nil {
t.Fatal(err)
}
if info.Size() != 0 {
t.Errorf("database size = %d, want 0 (not opened)", info.Size())
_, err := os.Stat(path)
if !errors.Is(err, fs.ErrNotExist) {
t.Errorf("stat %s = %v, want the database not created", path, err)
}
}
// assertSecondScanFails runs a scan against the database
// SFDUPES_DATABASE names while the test holds the lock on it. It fails
// the test unless that scan fails at once, naming the lock file beside
// path, the database's real path.
// 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)
@@ -883,9 +879,8 @@ func assertSecondScanFails(t *testing.T, path string) {
func TestRunSecondScanThroughSymlinkFails(t *testing.T) {
// README §Database: every path to one database names the same lock
// file, so while a scan holds it through one path, a scan through
// another fails, naming the lock file beside the database's real
// path.
// 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)
@@ -899,7 +894,6 @@ func TestRunSecondScanThroughSymlinkFails(t *testing.T) {
t.Fatal(err)
}
holdScanLock(t, path)
t.Setenv(databaseEnv, link)
assertSecondScanFails(t, path)
})
@@ -913,34 +907,9 @@ func TestRunSecondScanThroughSymlinkFails(t *testing.T) {
t.Fatal(err)
}
holdScanLock(t, path)
t.Setenv(databaseEnv, filepath.Join(link, filepath.Base(path)))
assertSecondScanFails(t, path)
})
t.Run("symlink whose target does not exist yet", func(t *testing.T) {
path := testDBPath(t)
link := filepath.Join(t.TempDir(), "link.sqlite")
err := os.Symlink(path, link)
if err != nil {
t.Fatal(err)
}
// The first scan takes the lock through the link before the
// database exists, then creates the database through it.
holdScanLock(t, link)
db, err := openScanDatabase(t.Context(), link)
if err != nil {
t.Fatal(err)
}
closeScanDatabase(t.Context(), db, link)
t.Setenv(databaseEnv, link)
assertSecondScanFails(t, path)
})
}
func TestRunScanReleasesLock(t *testing.T) {
-7
View File
@@ -139,13 +139,6 @@ func (p *progress) warnf(format string, args ...any) {
}
fmt.Fprintln(os.Stderr, msg)
// The library never draws a finished bar again, so the warning for
// a pass's last item would erase it for good: draw its last frame
// again below the warning.
if p.bar != nil && p.bar.IsFinished() {
fmt.Fprint(os.Stderr, p.bar.String())
}
}
// finish terminates the pass's display. A bar whose pass stopped short
-22
View File
@@ -187,25 +187,3 @@ func TestBarStoppedShortKeepsCount(t *testing.T) {
t.Errorf("terminal shows %q, want a count of 2 of 10", shown)
}
}
// TestWarningAfterLastItemKeepsBar checks that a warning printed after
// a pass's last item, as for a failed read of the last file, leaves the
// pass's finished bar on the terminal below the warning.
//
//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
func TestWarningAfterLastItemKeepsBar(t *testing.T) {
stderr := captureStderr(t)
p := &progress{
label: "hash", total: 2, start: time.Now(),
bar: newBar("hash", 2),
}
p.increment()
p.increment()
p.warnf("warning")
p.finish()
if shown := lastFrame(stderr()); !strings.Contains(shown, "(2/2,") {
t.Errorf("terminal shows %q, want a count of 2 of 2", shown)
}
}