fix(core): don't abort backup on dataless cloud-only directories - #184
Open
hexsprite wants to merge 1 commit into
Open
fix(core): don't abort backup on dataless cloud-only directories#184hexsprite wants to merge 1 commit into
hexsprite wants to merge 1 commit into
Conversation
On macOS, calling read_dir on a FileProvider dataless (iCloud cloud-only)
directory blocks fileproviderd trying to materialize the listing and
returns EDEADLK ("Resource deadlock avoided", os error 11). That error
propagated up and aborted the entire backup — no snapshot was written.
Skip descent into stat-detected dataless directories (record the dir
inode for restore, but never read_dir into it), mirroring the existing
dataless-file skip path. Also soft-classify EDEADLK on macOS in
is_soft_backup_io_error to cover the race where a directory flips
dataless between lstat and read_dir. Scoped to macOS so genuine
fcntl/flock deadlocks are not masked elsewhere.
Adds regression tests for the descent decision (should_descend_into_dir)
and the EDEADLK classification, both polarities.
Contributor
|
Thanks for the PR! Just wondering if we even need your change 1? Change 1 (unconditionally skipping descent into dataless directories) silently drops entire subtrees from the snapshot with only a debug! line — no dataless_skipped tally, no warning, and is_partial stays false, so a run that loses ~/Documents exits 0 and looks clean. That's worse than the file path it claims to mirror: dataless files get carried forward from ParentReuseIndex, whereas unreachable children are dropped outright, including ones already safely in the backup. Change 2 (soft-classifying EDEADLK) alone already fixes the reported abort and marks the snapshot partial correctly, so change 1 is both unnecessary for the bug and riskier than it. |
m3nu
added a commit
that referenced
this pull request
Jul 25, 2026
read_dir on a macOS FileProvider directory that cannot be materialized in this process context fails with EDEADLK (errno 11), which was not in the soft-error set, so the whole backup aborted with exit 1 and no snapshot — #183, reported against a source under iCloud Drive. is_fileprovider_deadlock classifies errno 11 as soft, scoped to macOS because 11 is EAGAIN on Linux and a real fcntl/flock deadlock on the BSDs, where masking it would hide genuine bugs. That alone stops the abort, and is the whole of PR #184's change 2. What it gets wrong is the accounting, which is the rest of this commit. An unlistable dataless directory would land in the generic soft-error bucket (stats.errors -> is_partial -> exit 3), turning every iCloud user's nightly job permanently red over content vykar deliberately never downloads. Dataless *file* skips had the opposite problem: invisible in the final report. Both are now tallied and reported, without touching the exit code. classify_dir_skip files a directory as dataless only when the errno is a FileProvider deadlock *and* the directory really is dataless, so an ordinary EACCES — or a genuine deadlock on macOS — stays a visible, counted soft error. The confirming lstat short-circuits behind the errno check, so it only ever fires on a path that has already failed. The tally rides the existing dataless plumbing (WalkEvent -> WalkEntry -> ProcessedEntry -> per-source counter) with a DatalessKind discriminant, so there is no new pipeline stage. The end-of-source summary gains a second clause, blunt on purpose: each skipped directory is an unbounded subtree missing from the snapshot, not one file. PR #184's change 1 — pre-emptive skip of descent into dataless directories — is deliberately not taken. It dropped whole subtrees on a bare debug! line with no tally and is_partial == false, so a run that lost ~/Documents exited 0 and looked clean. It is also broader than the bug: EDEADLK is raised when this process cannot drive fileproviderd, not by dataless-ness per se, and an interactive read_dir on a cloud-only directory generally lists fine. The counters are run-local, on BackupOutcome rather than SnapshotStats. SnapshotStats is a frozen 5-element positional msgpack type nested at envelope position 8, before the format_version discriminator; growing it would make new snapshots decode on older builds as corruption instead of as a clean newer-version refusal — the failure mode e85ab81 hardened against. A comment on the type records that so it is not re-litigated. Capturing this content rather than merely reporting it is the roadmap's per-source dataless: skip|hydrate|hydrate-evict, left untouched here so that work can define the knob in one place. Fixes #183 Refs #184
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #183.
Problem
On macOS,
std::fs::read_diron a FileProvider dataless (iCloud cloud-only) directory blocksfileproviderdand returnsEDEADLK—"Resource deadlock avoided" (os error 11). The walker treated that as fatal, so a single cloud-only directory aborted the entire backup (exit 1, no snapshot). A nightly backup silently stopped producing snapshots for weeks because of this.stat/lstaton a dataless inode succeeds instantly; only descent (read_dir) deadlocks — so the walker sails through the stat but dies the moment it tries to list the directory.Fix
should_descend_into_dir(actual_is_dir, is_symlink, is_dataless)inwalk/mod.rs. The directory inode is still recorded (restore recreates it); its cloud-only children are skipped — mirroring the existing dataless-file skip path.is_datalesswas already onMetadataSummary; it just wasn't consulted before descending.EDEADLKon macOS inis_soft_backup_io_error, covering the race where a directory flips dataless betweenlstatandread_dir. Scoped#[cfg(target_os = "macos")]so genuinefcntl/flockdeadlocks are not masked on other platforms.Tests
should_descend_into_dir— normal dir descends; dataless dir does not; symlink-dir does not; file does not.is_soft_backup_io_error— errno 11 is soft on macOS, not soft on other unix.make pre-commitclean (fmt + clippy-D warnings+ doc-check + tests).Verified in production
Full
vykar backup -R r2on the affected machine: snapshot created, 923,725 files, 0 deadlocks, 48,090 dataless files soft-skipped and tallied. First successful snapshot in weeks.Per CONTRIBUTING, this is filed against triaged issue #183 — happy to adjust approach if you'd prefer a different design before merge.