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
+25 -9
View File
@@ -113,8 +113,9 @@ Goals, in order:
`sfdupes`. `sfdupes`.
- Dependencies: standard library, `github.com/spf13/cobra` for the - Dependencies: standard library, `github.com/spf13/cobra` for the
CLI, **one progress-bar library** CLI, **one progress-bar library**
(`github.com/schollz/progressbar/v3`), and **one SQLite driver** (`github.com/schollz/progressbar/v3`), **one SQLite driver**
(`modernc.org/sqlite`, pure Go, so builds keep cgo disabled). (`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 `github.com/spf13/viper` is permitted if configuration-file support
is ever needed, but is not currently used. No other third-party is ever needed, but is not currently used. No other third-party
deps. 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 use. `report` and `trees` require an existing database; a missing
database file is a fatal error (exit 1) telling the user to run database file is a fatal error (exit 1) telling the user to run
`scan` first. `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 - 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 timeout, so running a report while a cron `scan` is in progress is
safe. The filesystem is authoritative; the database is an safe. The filesystem is authoritative; the database is an
@@ -567,9 +582,10 @@ Additional requirements:
### Error handling and exit codes ### Error handling and exit codes
- `0`: success, even if individual files were skipped with warnings. - `0`: success, even if individual files were skipped with warnings.
- `1`: fatal error (e.g., a `PATH` operand does not exist, the - `1`: fatal error (e.g., a `PATH` operand does not exist, another
database cannot be created/opened/read/written, a missing database `scan` is already running against the same database, the database
for `report`/`trees`, stdout write failure). 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 - `2`: usage error (including `scan` with no `PATH` operand and
`report`/`trees` with any positional argument). `report`/`trees` with any positional argument).
@@ -722,7 +738,7 @@ All of the following, run in this directory, must pass:
```sh ```sh
d=$(mktemp -d) d=$(mktemp -d)
export SFDUPES_DATABASE="$d/db.sqlite" export SFDUPES_DATABASE="$(mktemp -d)/db.sqlite"
mkdir -p "$d/a" "$d/b" mkdir -p "$d/a" "$d/b"
head -c 2000 /dev/urandom > "$d/a/one.bin" head -c 2000 /dev/urandom > "$d/a/one.bin"
cp "$d/a/one.bin" "$d/b/copy.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 ./sfdupes report
``` ```
(The scan database lives inside `$d` here purely for test hygiene; (The database lives in a temp directory of its own: inside `$d`,
scanning `$d` therefore also records the SQLite file itself, which the scan would record it, and its empty lock file would join the
is harmless.) `empty1`/`empty2` group.)
Expected from the first `report`: `one.bin`/`copy.bin`/`copy2.bin` Expected from the first `report`: `one.bin`/`copy.bin`/`copy2.bin`
form one group (two dupe rows, `first` is the lexicographically form one group (two dupe rows, `first` is the lexicographically
+4
View File
@@ -29,6 +29,10 @@
# Completed Steps # 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 - test stdout write failures in `report` and `trees`; README states that
`| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null` `| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null`
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30) (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30)
+47
View File
@@ -11,6 +11,7 @@ import (
"slices" "slices"
"strconv" "strconv"
"golang.org/x/sys/unix"
// The pure-Go SQLite driver, registered as "sqlite"; keeps cgo // The pure-Go SQLite driver, registered as "sqlite"; keeps cgo
// disabled. // disabled.
_ "modernc.org/sqlite" _ "modernc.org/sqlite"
@@ -32,6 +33,11 @@ const schemaVersion = 1
// scan. // scan.
const dbDirPerm = 0o755 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 // createTableSQL is the schema applied to a fresh database. Paths are
// BLOBs because Unix paths are raw bytes, not guaranteed UTF-8. // BLOBs because Unix paths are raw bytes, not guaranteed UTF-8.
const createTableSQL = ` const createTableSQL = `
@@ -64,6 +70,10 @@ var errNoDatabase = errors.New(
// does not understand. // does not understand.
var errSchemaVersion = errors.New("unsupported database schema version") 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 // databasePath resolves the database location: SFDUPES_DATABASE when
// set and non-empty, the compiled-in default otherwise. // set and non-empty, the compiled-in default otherwise.
func databasePath() string { func databasePath() string {
@@ -104,6 +114,43 @@ func openDB(path, params string) (*sql.DB, error) {
return db, nil 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 // openScanDatabase opens the database for the scan subcommand, creating
// the file, its parent directory, and the schema as needed. // the file, its parent directory, and the schema as needed.
func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) { func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
+1 -1
View File
@@ -5,6 +5,7 @@ go 1.25.7
require ( require (
github.com/schollz/progressbar/v3 v3.19.1 github.com/schollz/progressbar/v3 v3.19.1
github.com/spf13/cobra v1.10.2 github.com/spf13/cobra v1.10.2
golang.org/x/sys v0.46.0
modernc.org/sqlite v1.54.0 modernc.org/sqlite v1.54.0
) )
@@ -18,7 +19,6 @@ require (
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
github.com/rivo/uniseg v0.4.7 // indirect github.com/rivo/uniseg v0.4.7 // indirect
github.com/spf13/pflag v1.0.9 // 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 golang.org/x/term v0.44.0 // indirect
modernc.org/libc v1.74.1 // indirect modernc.org/libc v1.74.1 // indirect
modernc.org/mathutil v1.7.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) { func TestRunStdoutClosedIsFatal(t *testing.T) {
// README §Error handling: a stdout write failure exits 1, reported // README §Error handling: a stdout write failure exits 1, reported
// in one line on stderr. // 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 // 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 // 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 // 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 // parsing and the at-least-one-operand check are done by cobra. The
// are returned rather than exiting, so that the deferred close — which // scan holds the lock on the database for its whole run, so a second
// takes the database out of WAL mode — always runs. Cancelling ctx // scan fails before it walks the filesystem or opens the database.
// unwinds the worker pools and aborts the scan with the context's // Errors are returned rather than exiting, so that the deferred close —
// error. // 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, func runScan(ctx context.Context, roots []string, workers int,
oneFS bool, oneFS bool,
) error { ) error {
@@ -104,6 +106,13 @@ func runScan(ctx context.Context, roots []string, workers int,
dbPath := databasePath() dbPath := databasePath()
lock, err := lockScanDatabase(dbPath)
if err != nil {
return err
}
defer func() { _ = lock.Close() }()
db, err := openScanDatabase(ctx, dbPath) db, err := openScanDatabase(ctx, dbPath)
if err != nil { if err != nil {
return err return err