Name the scan lock file from the database's resolved path (closes #101) #104
@@ -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