fix(transfer): read the basis via windowed I/O to survive concurrent truncation - #7052
Merged
Merged
Conversation
…truncation The receiver dereferenced mmap'd basis pages on two paths - signature hashing (basis >= 64 KiB) and delta application (basis >= 1 MiB with the standard writer). If another process truncated the basis mid-read, the kernel raised SIGBUS while faulting the now-unmapped tail on our behalf, killing the process (exit 135 Linux / 138 Darwin) with no signal-safe recovery. Route both reads through the existing windowed read() reader that the io_uring path already used: delta application always opens the basis via BufferedMap, and signature hashing streams it through a BufReader sized to MAX_MAP_SIZE. A read(2) past a shrunk EOF returns short and surfaces as an ordinary io::Error - delta application reports it, and signature generation drops the basis and falls back to a whole-file transfer - exactly matching upstream, which uses read(2) for basis files for this reason (fileio.c:214-217 comment + map_ptr()). Byte output is unchanged for the non-truncated case; the bytes and per-block sums are identical. Adds regression tests that truncate the basis after it is opened and assert a graceful io::Error / whole-file fallback instead of a signal death, on both the signature and delta-apply paths.
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
1 participant
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.
What
Fixes a receiver crash: when the basis file was truncated by another process mid-read, oc-rsync dereferenced mmap'd basis pages and died by SIGBUS (exit 135 Linux / 138 Darwin) with no signal-safe recovery. Upstream deliberately uses windowed
read(2)on the basis for exactly this reason (fileio.c:214-217,map_ptr()): a short read degrades gracefully instead of faulting.Root cause
Two receiver paths dereferenced the mmap:
delta_apply/applicator.rs) — Standard writer kind, basis >= 1 MiBreceiver/basis.rs) — basis >= 64 KiB, regardless of writer kindThe io_uring delta path already dodged this via
open_adaptive_buffered; the default Standard path and the signature path were still exposed.Fix
Route both basis reads through the existing windowed reader (no
unsafe), mirroring upstream:applicator.rs: basis is now always opened viaMapFile::open_adaptive_buffered(theis_io_uring()special-case is gone); a truncated file yieldsUnexpectedEof, which delta-apply reports.basis.rs:generate_basis_signaturereads viaBufReader; a truncation yields the whole-file fallback (BasisFileResult::EMPTY). DeadMMAP_BASIS_THRESHOLD_BYTES+reopen_basisremoved.Non-truncated output is byte-identical (same bytes, same per-block sums, same order).
Tests
Two regression tests that SIGBUS pre-fix and pass now:
basis_apply_survives_concurrent_truncation,generate_basis_signature_survives_concurrent_truncation. Existing mmap-parity tests rewritten to the windowed reader.Perf
Direct A/B (512 MiB basis, warm cache, every-byte scan): windowed vs mmap = 1.03-1.06x (3-6% slower in the microbench worst case), throughput ~170-190 MB/s for both - dominated by per-byte work, not the read strategy. Not meaningful end-to-end (strong-checksum + network + disk-write dwarf it). A follow-up
fast_io-hosted SIGBUS-guarded mmap could recover the microbench delta if ever warranted.