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 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.
|
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`
|
- 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
|
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 appending `.lock` to the database's path with symlinks resolved
|
||||||
by default), taken before it walks the filesystem or opens the database. A
|
(`/var/lib/sfdupes/db.sqlite.lock` by default), taken before it walks the
|
||||||
second `scan` against the same database does not wait: it fails at once with a
|
filesystem or opens the database. Every path to one database, through a
|
||||||
one-line error naming the lock file and exits 1, without walking anything or
|
symlink to the file or to a directory above it, names the same lock file. Just
|
||||||
opening the database, and the running scan carries on. The lock file is
|
before it takes the lock, `scan` creates the database file, empty, if it is
|
||||||
created on first use, open to its owner only, and left in place: a leftover
|
missing (through a symlink at the database path, it creates the symlink's
|
||||||
file blocks nothing, because the lock ends with the process holding it however
|
target), so the lock file is named the same way before the first scan sets up
|
||||||
it ends, a fatal error or an interrupt included, and deleting the file while a
|
the database as after. A second `scan` against the same database does not
|
||||||
scan runs would let a second scan start. `report` and `trees` never take the
|
wait: it fails at once with a one-line error naming the lock file and exits 1,
|
||||||
lock, so they run during a scan.
|
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
|
- 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
|
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.
|
authoritative; the database is an eventually-consistent reflection of it.
|
||||||
|
|||||||
@@ -29,6 +29,11 @@
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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,
|
phase's finished bar: the bar is drawn again below the warning (2026-10-14,
|
||||||
https://git.eeqj.de/sneak/sfdupes/issues/102)
|
https://git.eeqj.de/sneak/sfdupes/issues/102)
|
||||||
|
|||||||
@@ -35,6 +35,10 @@ const schemaVersion = 1
|
|||||||
// scan.
|
// scan.
|
||||||
const dbDirPerm = 0o755
|
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
|
// 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
|
// the file can hold the lock and keep every scan from running, so it
|
||||||
// is open to its owner only.
|
// 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
|
// lockScanDatabase takes the lock that keeps a second scan off the
|
||||||
// database at path: an exclusive flock(2) on the file beside it named
|
// database at path: an exclusive flock(2) on the file beside it named
|
||||||
// path with ".lock" appended, created along with the database's parent
|
// by the database's path with symlinks resolved and ".lock" appended,
|
||||||
// directory if missing. A lock held by another scan fails at once
|
// so every path to one database names the same lock file. If missing,
|
||||||
// instead of waiting. The lock lasts until the returned file is closed
|
// the database's parent directory and an empty database file are
|
||||||
// or the process ends. The file is never deleted: a scan that deleted
|
// created first, so the lock file is named the same way before the
|
||||||
// it would let the next scan lock a new file while another still holds
|
// database is set up as after. A lock held by another scan fails at
|
||||||
// the old one.
|
// 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) {
|
func lockScanDatabase(path string) (*os.File, error) {
|
||||||
err := os.MkdirAll(filepath.Dir(path), dbDirPerm)
|
err := os.MkdirAll(filepath.Dir(path), dbDirPerm)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("create database directory: %w", err)
|
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
|
//nolint:gosec // the operator chooses the database path
|
||||||
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)
|
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)
|
||||||
|
|||||||
+10
-2
@@ -13,11 +13,19 @@ import (
|
|||||||
"time"
|
"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 {
|
func testDBPath(t *testing.T) string {
|
||||||
t.Helper()
|
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.
|
// 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) {
|
func TestRunSecondScanFails(t *testing.T) {
|
||||||
// README §Database: while one scan holds the lock, a second scan
|
// 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.
|
// database.
|
||||||
path := testDBPath(t)
|
path := testDBPath(t)
|
||||||
t.Setenv(databaseEnv, path)
|
t.Setenv(databaseEnv, path)
|
||||||
@@ -848,10 +848,99 @@ func TestRunSecondScanFails(t *testing.T) {
|
|||||||
t.Errorf("stdout = %q, want nothing (data only)", got)
|
t.Errorf("stdout = %q, want nothing (data only)", got)
|
||||||
}
|
}
|
||||||
|
|
||||||
_, err := os.Stat(path)
|
// Taking the lock created the database file, empty; opening it as
|
||||||
if !errors.Is(err, fs.ErrNotExist) {
|
// a database would have written to it.
|
||||||
t.Errorf("stat %s = %v, want the database not created", path, err)
|
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) {
|
func TestRunScanReleasesLock(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user