Hold a lock so a second scan fails at once (closes #53)
check / check (push) Successful in 1m43s
check / check (push) Successful in 1m43s
scan takes an exclusive flock(2) on a lock file beside the database (its path with .lock appended) before it walks anything or opens the database, and holds it until it returns. A second scan against the same database fails at once with a one-line error naming the lock file and exits 1. report and trees never take the lock. The lock ends with the process, so a fatal error or an interrupt releases it; the file is never deleted. golang.org/x/sys becomes a direct dependency. The README smoke test now keeps the database outside the scanned tree, where its empty lock file would have joined the empty-file group. Model: opus-5-5
This commit was merged in pull request #75.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user