Hold a lock so a second scan fails at once (closes #53) #75
@@ -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
|
||||||
|
|||||||
@@ -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)
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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.
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
Reference in New Issue
Block a user