Open files to hash without blocking and reject non-regular ones (closes #100)
check / check (push) Waiting to run
check / check (push) Waiting to run
A regular file replaced by a FIFO after the walk statted it made the hash phase's open wait forever for a writer, so the scan never ended and kept the lock. The hash and content phases now open each file with O_NONBLOCK and check with fstat that it is still a regular file; anything else is a per-path error, warned about and skipped. The new test replaces a walked file with a FIFO before the hash phase reads it. Model: opus-5-5
This commit was merged in pull request #107.
This commit is contained in:
@@ -29,6 +29,11 @@
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- the hash and content phases open each file without blocking and check that it
|
||||
is still a regular file, so one replaced by a FIFO or device node after it was
|
||||
statted is warned about and skipped instead of hanging the scan (2026-10-14,
|
||||
https://git.eeqj.de/sneak/sfdupes/issues/100)
|
||||
|
||||
- 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,
|
||||
|
||||
@@ -1309,6 +1309,38 @@ func hashWorker(ctx context.Context, jobs <-chan []fileRec,
|
||||
}
|
||||
}
|
||||
|
||||
// errNotRegular reports a path that is no longer a regular file when it
|
||||
// is opened to be read.
|
||||
var errNotRegular = errors.New("not a regular file")
|
||||
|
||||
// openRegularFile opens path for reading and returns it only if it is
|
||||
// still a regular file, since the path may have been replaced after it
|
||||
// was statted. O_NONBLOCK keeps the open of a FIFO or a device node
|
||||
// from waiting forever, and fstat then rejects it. Reads of a regular
|
||||
// file ignore O_NONBLOCK.
|
||||
func openRegularFile(path string) (*os.File, error) {
|
||||
//nolint:gosec // hashing operator-supplied paths is the tool's purpose
|
||||
f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
fi, err := f.Stat()
|
||||
if err != nil {
|
||||
_ = f.Close()
|
||||
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if !fi.Mode().IsRegular() {
|
||||
_ = f.Close()
|
||||
|
||||
return nil, errNotRegular
|
||||
}
|
||||
|
||||
return f, nil
|
||||
}
|
||||
|
||||
// emptyHash is the lowercase-hex SHA-256 of the empty input: the head,
|
||||
// tail, and content hash of every zero-length file.
|
||||
const emptyHash = "e3b0c44298fc1c149afbf4c8996fb924" +
|
||||
@@ -1331,8 +1363,7 @@ func hashSignature(path string, size int64) (string, string, string, error) {
|
||||
return emptyHash, emptyHash, emptyHash, nil
|
||||
}
|
||||
|
||||
//nolint:gosec // hashing operator-supplied paths is the tool's purpose
|
||||
f, err := os.Open(path)
|
||||
f, err := openRegularFile(path)
|
||||
if err != nil {
|
||||
return "", "", "", err
|
||||
}
|
||||
@@ -1363,8 +1394,7 @@ func hashSignature(path string, size int64) (string, string, string, error) {
|
||||
// tail are returned empty, because the content phase keeps the ones its
|
||||
// records already hold.
|
||||
func hashContentOnly(path string, size int64) (string, string, string, error) {
|
||||
//nolint:gosec // hashing operator-supplied paths is the tool's purpose
|
||||
f, err := os.Open(path)
|
||||
f, err := openRegularFile(path)
|
||||
if err != nil {
|
||||
return "", "", "", err
|
||||
}
|
||||
|
||||
@@ -1797,6 +1797,54 @@ func TestScanHardlinkRunFailsTogether(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestHashPhaseSkipsFileReplacedByFIFO replaces a walked regular file
|
||||
// with a FIFO before the hash phase reads it. Opening a FIFO that has
|
||||
// no writer waits for one, so a blocking open would hang the phase. The
|
||||
// path must be warned about once and skipped, with no record written.
|
||||
//
|
||||
//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
|
||||
func TestHashPhaseSkipsFileReplacedByFIFO(t *testing.T) {
|
||||
stderr := captureStderr(t)
|
||||
dir := t.TempDir()
|
||||
p := writeFile(t, dir, "a.bin", pattern(1, 300))
|
||||
|
||||
walked, _ := collectWalk(t, []string{dir}, false, 1)
|
||||
|
||||
err := os.Remove(p)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err = unix.Mkfifo(p, 0o600)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
s := &scanState{db: openTestDB(t), toHash: walked}
|
||||
|
||||
err = s.hashPhase(t.Context(), 1)
|
||||
if err != nil {
|
||||
t.Fatalf("hashPhase = %v, want nil", err)
|
||||
}
|
||||
|
||||
if s.st.skipped != 1 || len(s.batch) != 0 {
|
||||
t.Errorf("skipped = %d, records = %d, want 1 skipped and none",
|
||||
s.st.skipped, len(s.batch))
|
||||
}
|
||||
|
||||
warning := "hash " + p + ": not a regular file\n"
|
||||
if got := stderr(); strings.Count(got, warning) != 1 {
|
||||
t.Errorf("stderr = %q, want %q once", got, warning)
|
||||
}
|
||||
|
||||
// The content phase opens files the same way.
|
||||
_, _, content, err := hashContentOnly(p, headTailMin)
|
||||
if content != "" || !errors.Is(err, errNotRegular) {
|
||||
t.Errorf("hashContentOnly on a FIFO = %q, %v; want no hash and %v",
|
||||
content, err, errNotRegular)
|
||||
}
|
||||
}
|
||||
|
||||
// injectedWriteFailure is the message the injected database trigger
|
||||
// aborts with, so the test can recognize its own failure in the error
|
||||
// the scan reports.
|
||||
|
||||
Reference in New Issue
Block a user