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. Before it takes the lock, scan now creates the database file, empty, if it is missing, following a symlink to its target as SQLite would, and names the lock file from that file's path with symlinks resolved. A scan started before the database exists therefore takes the same lock file as every later scan through the same path. 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 was merged in pull request #104.
This commit is contained in:
@@ -271,16 +271,21 @@ 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
|
||||
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.
|
||||
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.
|
||||
- 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.
|
||||
|
||||
@@ -29,6 +29,11 @@
|
||||
|
||||
# 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)
|
||||
|
||||
@@ -35,6 +35,10 @@ 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.
|
||||
@@ -140,19 +144,37 @@ 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
|
||||
// the old one.
|
||||
// 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.
|
||||
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)
|
||||
}
|
||||
|
||||
lockPath := path + ".lock"
|
||||
// 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)
|
||||
}
|
||||
|
||||
_ = dbFile.Close()
|
||||
|
||||
resolved, err := filepath.EvalSymlinks(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.
|
||||
|
||||
+93
-4
@@ -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 creating the
|
||||
// fails at once, naming the lock file, without opening the
|
||||
// database.
|
||||
path := testDBPath(t)
|
||||
t.Setenv(databaseEnv, path)
|
||||
@@ -848,10 +848,99 @@ func TestRunSecondScanFails(t *testing.T) {
|
||||
t.Errorf("stdout = %q, want nothing (data only)", got)
|
||||
}
|
||||
|
||||
_, err := os.Stat(path)
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
t.Errorf("stat %s = %v, want the database not created", path, err)
|
||||
// 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())
|
||||
}
|
||||
}
|
||||
|
||||
// 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.
|
||||
func assertSecondScanFails(t *testing.T, path string) {
|
||||
t.Helper()
|
||||
|
||||
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 one path, a scan through
|
||||
// another fails, naming the lock file beside the database's real
|
||||
// path.
|
||||
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)
|
||||
}
|
||||
|
||||
holdScanLock(t, path)
|
||||
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)
|
||||
}
|
||||
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user