Hold a lock so a second scan fails at once (closes #53) #75

Merged
clawbot merged 1 commits from issue-53-scan-lock into next 2026-10-04 02:30:23 +02:00
6 changed files with 180 additions and 15 deletions
Showing only changes of commit efc2b62a77 - Show all commits
+25 -9
View File
@@ -113,8 +113,9 @@ Goals, in order:
`sfdupes`.
- Dependencies: standard library, `github.com/spf13/cobra` for the
CLI, **one progress-bar library**
(`github.com/schollz/progressbar/v3`), and **one SQLite driver**
(`modernc.org/sqlite`, pure Go, so builds keep cgo disabled).
(`github.com/schollz/progressbar/v3`), **one SQLite driver**
(`modernc.org/sqlite`, pure Go, so builds keep cgo disabled), and
`golang.org/x/sys` for `flock(2)` (the scan lock, see "Database").
`github.com/spf13/viper` is permitted if configuration-file support
is ever needed, but is not currently used. No other third-party
deps.
@@ -152,6 +153,20 @@ All three subcommands operate on a single SQLite database file:
use. `report` and `trees` require an existing database; a missing
database file is a fatal 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.
- 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
@@ -567,9 +582,10 @@ Additional requirements:
### Error handling and exit codes
- `0`: success, even if individual files were skipped with warnings.
- `1`: fatal error (e.g., a `PATH` operand does not exist, the
database cannot be created/opened/read/written, a missing database
for `report`/`trees`, stdout write failure).
- `1`: fatal error (e.g., a `PATH` operand does not exist, another
`scan` is already running against the same database, the database
cannot be created/opened/read/written, a missing database for
`report`/`trees`, stdout write failure).
- `2`: usage error (including `scan` with no `PATH` operand and
`report`/`trees` with any positional argument).
@@ -722,7 +738,7 @@ All of the following, run in this directory, must pass:
```sh
d=$(mktemp -d)
export SFDUPES_DATABASE="$d/db.sqlite"
export SFDUPES_DATABASE="$(mktemp -d)/db.sqlite"
mkdir -p "$d/a" "$d/b"
head -c 2000 /dev/urandom > "$d/a/one.bin"
cp "$d/a/one.bin" "$d/b/copy.bin"
@@ -750,9 +766,9 @@ All of the following, run in this directory, must pass:
./sfdupes report
```
(The scan database lives inside `$d` here purely for test hygiene;
scanning `$d` therefore also records the SQLite file itself, which
is harmless.)
(The database lives in a temp directory of its own: inside `$d`,
the scan would record it, and its empty lock file would join the
`empty1`/`empty2` group.)
Expected from the first `report`: `one.bin`/`copy.bin`/`copy2.bin`
form one group (two dupe rows, `first` is the lexicographically
+4
View File
@@ -29,6 +29,10 @@
# Completed Steps
- `scan` holds a lock on a lock file beside the database for its whole run,
so a second `scan` fails at once with exit 1 (2026-10-03,
https://git.eeqj.de/sneak/sfdupes/issues/53)
- test stdout write failures in `report` and `trees`; README states that
`| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null`
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30)
+47
View File
@@ -11,6 +11,7 @@ import (
"slices"
"strconv"
"golang.org/x/sys/unix"
// The pure-Go SQLite driver, registered as "sqlite"; keeps cgo
// disabled.
_ "modernc.org/sqlite"
@@ -32,6 +33,11 @@ const schemaVersion = 1
// scan.
const dbDirPerm = 0o755
// 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.
const lockFilePerm = 0o600
// createTableSQL is the schema applied to a fresh database. Paths are
// BLOBs because Unix paths are raw bytes, not guaranteed UTF-8.
const createTableSQL = `
@@ -64,6 +70,10 @@ var errNoDatabase = errors.New(
// does not understand.
var errSchemaVersion = errors.New("unsupported database schema version")
// errScanRunning reports that another scan holds the lock on the
// database.
var errScanRunning = errors.New("another scan is running")
// databasePath resolves the database location: SFDUPES_DATABASE when
// set and non-empty, the compiled-in default otherwise.
func databasePath() string {
@@ -104,6 +114,43 @@ func openDB(path, params string) (*sql.DB, error) {
return db, nil
}
// 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.
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"
//nolint:gosec // the operator chooses the database path
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)
if err != nil {
return nil, err
}
err = unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)
if err != nil {
_ = f.Close()
if errors.Is(err, unix.EWOULDBLOCK) {
return nil, fmt.Errorf("%w (lock held on %s)",
errScanRunning, lockPath)
}
return nil, fmt.Errorf("lock %s: %w", lockPath, err)
}
return f, nil
}
// openScanDatabase opens the database for the scan subcommand, creating
// the file, its parent directory, and the schema as needed.
func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
+1 -1
View File
@@ -5,6 +5,7 @@ go 1.25.7
require (
github.com/schollz/progressbar/v3 v3.19.1
github.com/spf13/cobra v1.10.2
golang.org/x/sys v0.46.0
modernc.org/sqlite v1.54.0
)
@@ -18,7 +19,6 @@ require (
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/spf13/pflag v1.0.9 // indirect
golang.org/x/sys v0.46.0 // indirect
golang.org/x/term v0.44.0 // indirect
modernc.org/libc v1.74.1 // indirect
modernc.org/mathutil v1.7.1 // indirect
+89
View File
@@ -412,6 +412,95 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
}
}
// holdScanLock takes the lock on the database at path, as a running
// scan does, and holds it until the test ends. It fails the test when
// the lock is already held.
func holdScanLock(t *testing.T, path string) {
t.Helper()
lock, err := lockScanDatabase(path)
if err != nil {
t.Fatalf("lock %s: %v", path, err)
}
t.Cleanup(func() { _ = lock.Close() })
}
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
// database.
path := testDBPath(t)
t.Setenv(databaseEnv, path)
holdScanLock(t, path)
var stdout, stderr bytes.Buffer
code := run([]string{cmdScan, t.TempDir()}, &stdout, &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)
}
if got := stdout.String(); got != "" {
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)
}
}
func TestRunScanReleasesLock(t *testing.T) {
// README §Database: a scan releases the lock however it ends.
t.Run("success", func(t *testing.T) {
path := testDBPath(t)
t.Setenv(databaseEnv, path)
scanFixture(t)
holdScanLock(t, path)
})
t.Run("fatal error", func(t *testing.T) {
path := brokenDatabase(t)
t.Setenv(databaseEnv, path)
code := run([]string{cmdScan, t.TempDir()}, io.Discard, io.Discard)
if code != exitFatal {
t.Fatalf("run(scan) = %d, want %d", code, exitFatal)
}
holdScanLock(t, path)
})
}
func TestRunReportsDuringScan(t *testing.T) {
// README §Database: report and trees never take the lock, so they
// run while a scan holds it.
path := testDBPath(t)
t.Setenv(databaseEnv, path)
scanFixture(t)
holdScanLock(t, path)
for _, name := range []string{cmdReport, cmdTrees} {
var stderr bytes.Buffer
code := run([]string{name}, io.Discard, &stderr)
if code != exitOK {
t.Errorf("run(%s) = %d, want %d; stderr: %s",
name, code, exitOK, stderr.String())
}
}
}
func TestRunStdoutClosedIsFatal(t *testing.T) {
// README §Error handling: a stdout write failure exits 1, reported
// in one line on stderr.
+14 -5
View File
@@ -85,11 +85,13 @@ type fileMeta struct {
// least one other file shares are ever hashed: a size-unique file
// cannot be a duplicate. A file of headTailMin or more gets its content
// hash only when its size, head, and tail match another file's. Flag
// parsing and the at-least-one-operand check are done by cobra. Errors
// are returned rather than exiting, so that the deferred close — which
// takes the database out of WAL mode — always runs. Cancelling ctx
// unwinds the worker pools and aborts the scan with the context's
// error.
// parsing and the at-least-one-operand check are done by cobra. The
// scan holds the lock on the database for its whole run, so a second
// scan fails before it walks the filesystem or opens the database.
// Errors are returned rather than exiting, so that the deferred close —
// which takes the database out of WAL mode — always runs, and the lock
// is released after it. Cancelling ctx unwinds the worker pools and
// aborts the scan with the context's error.
func runScan(ctx context.Context, roots []string, workers int,
oneFS bool,
) error {
@@ -104,6 +106,13 @@ func runScan(ctx context.Context, roots []string, workers int,
dbPath := databasePath()
lock, err := lockScanDatabase(dbPath)
if err != nil {
return err
}
defer func() { _ = lock.Close() }()
db, err := openScanDatabase(ctx, dbPath)
if err != nil {
return err