diff --git a/crates/transfer/src/delta_apply/applicator.rs b/crates/transfer/src/delta_apply/applicator.rs index b01847d2e..a896d7519 100644 --- a/crates/transfer/src/delta_apply/applicator.rs +++ b/crates/transfer/src/delta_apply/applicator.rs @@ -72,36 +72,32 @@ type BasisMapStrategy = BufferedMap; /// Kind of writer paired with the [`DeltaApplicator`]. /// -/// Drives the basis-file mapping policy: when the writer is io_uring-backed, -/// the basis file must be opened via [`crate::map_file::BufferedMap`] (a sliding-window -/// `pread(2)` reader) rather than `mmap(2)`. Submitting an `mmap`-backed -/// pointer to an `io_uring` SQE has two failure modes: +/// The basis file is always opened via [`crate::map_file::BufferedMap`] (a +/// sliding-window `read(2)` reader) rather than `mmap(2)`, regardless of the +/// writer kind. Memory-mapping the basis has two failure modes this avoids: /// -/// 1. Cold-page faults are serviced under the SQE submission thread (or the -/// SQPOLL kernel thread when SQPOLL is enabled), turning a "free" zero-copy -/// write into a synchronous fault and stalling other in-flight SQEs on the -/// same poller. -/// 2. A concurrent truncation of the basis file raises `SIGBUS` while the -/// kernel is dereferencing the page on our behalf - recovery from -/// in-kernel `SIGBUS` is not signal-safe. +/// 1. A concurrent truncation of the basis file raises `SIGBUS` while the +/// kernel is dereferencing the now-unmapped page on our behalf - recovery +/// from in-kernel `SIGBUS` is not signal-safe. A windowed `read(2)` past +/// the shrunk EOF instead returns short and surfaces as an `io::Error`. +/// 2. When paired with an io_uring writer, cold-page faults on an mmap-backed +/// pointer submitted in an SQE are serviced under the SQE submission thread +/// (or the SQPOLL kernel thread), turning a "free" zero-copy write into a +/// synchronous fault and stalling other in-flight SQEs on the same poller. /// /// Upstream rsync deliberately avoids `mmap(2)` for basis files for the same -/// truncation reason - see `fileio.c:214-217` in upstream rsync 3.4.1. +/// truncation reason - see `fileio.c:214-217` + `map_ptr()` in upstream rsync. /// /// See `docs/design/basis-file-io-policy.md` and audit -/// `docs/audits/mmap-iouring-co-usage.md` finding F1. +/// `docs/audits/mmap-iouring-co-usage.md` finding F1. The variant is retained +/// for callers and future policy (e.g. a SIGBUS-guarded mmap fast path) but no +/// longer changes the basis read strategy. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum BasisWriterKind { /// Standard buffered writer (or any writer not backed by io_uring). - /// - /// On Unix the basis file is opened with the adaptive strategy, which - /// selects `mmap(2)` for files >= 1 MiB. This matches existing behaviour. #[default] Standard, /// io_uring-backed writer (e.g. `IoUringWriter` or `IoUringDiskBatch`). - /// - /// Forces the basis file onto [`crate::map_file::BufferedMap`] regardless of size to keep - /// `mmap`-backed pointers out of any io_uring submission queue entry. IoUring, } @@ -169,19 +165,17 @@ pub struct DeltaApplyResult { /// /// # Performance Optimizations /// -/// - Uses `MapFile` with `BasisMapStrategy` for basis file access. The -/// exact strategy is policy-driven via [`DeltaApplyConfig::writer_kind`]: -/// - Unix, [`BasisWriterKind::Standard`]: `AdaptiveMapStrategy` - -/// files < 1MB use buffered I/O (256KB sliding window), files >= 1MB -/// use mmap for zero-copy access. -/// - Unix, [`BasisWriterKind::IoUring`]: forced to `BufferedMap` for all -/// sizes. Submitting an mmap-backed pointer to an io_uring SQE can -/// stall the SQPOLL kernel thread on cold-page faults and raises -/// `SIGBUS` on concurrent truncation. Mirrors upstream rsync's -/// deliberate avoidance of `mmap(2)` for basis files -/// (`fileio.c:214-217`). See `docs/design/basis-file-io-policy.md`. -/// - Non-Unix: `BufferedMap` - buffered I/O with 256KB sliding window -/// for all files. +/// - Uses `MapFile` for basis file access via a 256KB sliding-window +/// `read(2)` reader (`BufferedMap`) on every platform and for every +/// [`BasisWriterKind`]. The basis is never memory-mapped: a concurrent +/// truncation by another process would raise `SIGBUS` when the kernel +/// faults the unmapped tail on our behalf, whereas a windowed `read(2)` +/// past the shrunk EOF returns short and surfaces as an ordinary +/// `io::Error`. Windowed reads also keep mmap-backed pointers out of any +/// io_uring SQE, whose cold-page faults would stall the SQPOLL kernel +/// thread. Mirrors upstream rsync's deliberate use of `read(2)` for basis +/// files (`fileio.c:214-217` + `map_ptr()`). See +/// `docs/design/basis-file-io-policy.md`. /// - Uses `TokenBuffer` for literal data, reusing the same allocation across /// all tokens to avoid per-token heap allocations. pub struct DeltaApplicator<'a> { @@ -212,18 +206,15 @@ impl<'a> DeltaApplicator<'a> { /// Creates a new delta applicator. /// /// If `basis_path` is provided, opens the file once and caches it for - /// efficient block reference lookups. Basis-file mapping policy: - /// - /// - **Unix, standard writer**: `AdaptiveMapStrategy` - mmap for files - /// >= 1 MiB, buffered for smaller (existing behaviour). - /// - **Unix, io_uring writer**: forces `BufferedMap` regardless of size. - /// Mmap'd basis pointers must never reach an io_uring SQE: cold-page - /// faults stall the SQPOLL kernel thread, and truncation by another - /// process raises `SIGBUS` inside the kernel SQE service path. Mirrors - /// upstream rsync's `fileio.c:214-217` rationale for using `read(2)` - /// instead of `mmap(2)` on basis files. See - /// `docs/design/basis-file-io-policy.md`. - /// - **Non-Unix**: always `BufferedMap` (no mmap path is wired). + /// efficient block reference lookups. The basis is always read through a + /// 256KB sliding-window `read(2)` reader (`BufferedMap`), never memory + /// mapped, so a concurrent truncation of the basis by another process + /// surfaces as an ordinary short-read `io::Error` instead of a `SIGBUS` + /// the kernel raises while faulting an unmapped page on our behalf. This + /// also keeps mmap-backed pointers out of any io_uring SQE. Mirrors + /// upstream rsync's `fileio.c:214-217` rationale for using `read(2)` + /// instead of `mmap(2)` on basis files. See + /// `docs/design/basis-file-io-policy.md`. pub fn new( output: File, config: &DeltaApplyConfig, @@ -232,21 +223,23 @@ impl<'a> DeltaApplicator<'a> { basis_path: Option<&'a Path>, ) -> io::Result { let basis_map = if let Some(path) = basis_path { + // Always read the basis through the windowed read() reader, never + // mmap. If another process truncates the basis mid-transfer (the + // canonical case upstream cites: a mailer rewriting the file), a + // dereference of the now-unmapped tail raises SIGBUS while the + // kernel is faulting the page in on our behalf - unrecoverable + // without a signal handler. A read(2) past the shrunk EOF instead + // returns short and surfaces as an ordinary io::Error the receiver + // reports as a transfer failure. Buffered reads also keep + // mmap-backed pointers out of any io_uring SQE, whose cold-page + // faults would otherwise stall the SQPOLL kernel thread. + // upstream: fileio.c:214-217 comment + map_ptr() deliberately use + // read(2) instead of mmap(2) on basis files for this exact reason. + let _ = config.writer_kind; #[cfg(unix)] - let map = if config.writer_kind.is_io_uring() { - // Avoid mmap when paired with io_uring (#1906, audit F1). - MapFile::open_adaptive_buffered(path) - } else { - MapFile::open_adaptive(path) - }; + let map = MapFile::open_adaptive_buffered(path); #[cfg(not(unix))] - let map = { - // BufferedMap is the only basis strategy on non-Unix; the - // io_uring path itself is Linux-only, so the writer_kind - // signal is consumed indirectly via the cfg gate. - let _ = config.writer_kind; - MapFile::::open(path) - }; + let map = MapFile::::open(path); Some(map.map_err(|e| { io::Error::new(e.kind(), format!("failed to open basis file {path:?}: {e}")) @@ -271,9 +264,10 @@ impl<'a> DeltaApplicator<'a> { /// Returns true if a basis file is open and is using the mmap strategy. /// - /// Used by tests to verify the policy decision in - /// [`Self::new`]: an io_uring-backed writer must never produce a - /// mmap-backed basis. See `docs/design/basis-file-io-policy.md`. + /// Used by tests to verify the policy decision in [`Self::new`]: the basis + /// is always opened through the windowed `BufferedMap` reader, so this must + /// return `false` for every writer kind and file size. See + /// `docs/design/basis-file-io-policy.md`. #[must_use] pub fn basis_uses_mmap(&self) -> bool { #[cfg(unix)] @@ -319,15 +313,12 @@ impl<'a> DeltaApplicator<'a> { /// Applies a block reference by copying from basis file. /// - /// Uses the cached `MapFile` opened in [`Self::new`]. The mapping - /// strategy depends on the configured [`BasisWriterKind`]: - /// - Standard writer (Unix): adaptive - 256KB sliding window for files - /// < 1MB, zero-copy mmap for files >= 1MB. - /// - io_uring writer: 256KB sliding window via `BufferedMap`, - /// regardless of size, to keep mmap pointers out of any io_uring SQE - /// (audit `docs/audits/mmap-iouring-co-usage.md` finding F1; upstream - /// `fileio.c:214-217`). - /// - Non-Unix: 256KB sliding window for all files. + /// Uses the cached `MapFile` opened in [`Self::new`], which always reads + /// the basis through a 256KB sliding-window `read(2)` reader + /// (`BufferedMap`) on every platform. The basis is never memory-mapped so + /// a concurrent truncation cannot raise `SIGBUS`, and no mmap pointer can + /// reach an io_uring SQE (audit `docs/audits/mmap-iouring-co-usage.md` + /// finding F1; upstream `fileio.c:214-217` + `map_ptr()`). /// /// # Errors /// @@ -983,8 +974,8 @@ mod tests { } /// Creates a 2 MiB basis file and an output file, returning paths. - /// 2 MiB is above `MMAP_THRESHOLD` (1 MiB) so the adaptive strategy - /// would otherwise pick mmap on Unix. + /// 2 MiB is above the legacy `MMAP_THRESHOLD` (1 MiB); the applicator must + /// still open it via the windowed `BufferedMap` reader, never mmap. fn make_large_basis(dir: &tempfile::TempDir) -> (std::path::PathBuf, File) { let basis_path = dir.path().join("basis.bin"); let out_path = dir.path().join("out.bin"); @@ -1000,7 +991,7 @@ mod tests { } #[test] - fn standard_writer_kind_uses_mmap_on_unix_for_large_basis() { + fn standard_writer_kind_never_mmaps_large_basis() { let dir = tempdir().expect("tempdir"); let (basis_path, out) = make_large_basis(&dir); let config = DeltaApplyConfig { @@ -1013,16 +1004,16 @@ mod tests { DeltaApplicator::new(out, &config, verifier, None, Some(basis_path.as_path())) .expect("construct applicator"); assert!(applicator.has_basis()); - // On Unix, AdaptiveMapStrategy picks mmap for files >= 1 MiB. - // On non-Unix only BufferedMap exists, so basis_uses_mmap() is - // always false. - #[cfg(unix)] + // The load-bearing correctness invariant: even a 2 MiB basis (above + // the legacy 1 MiB mmap threshold) with the standard writer must use + // the windowed read() reader, never mmap. Memory-mapping the basis + // would raise SIGBUS if another process truncates it mid-transfer; + // read(2) past the shrunk EOF surfaces as an ordinary io::Error + // instead (upstream fileio.c:214-217 + map_ptr()). assert!( - applicator.basis_uses_mmap(), - "standard writer + 2 MiB basis should pick mmap on Unix" + !applicator.basis_uses_mmap(), + "standard writer must read the basis via windowed read(), never mmap" ); - #[cfg(not(unix))] - assert!(!applicator.basis_uses_mmap()); } #[test] @@ -1066,6 +1057,93 @@ mod tests { assert!(!applicator.basis_uses_mmap()); } + /// Regression (#277): a basis truncated by another process after the + /// applicator opened it must surface as an ordinary `io::Error`, never a + /// `SIGBUS` that kills the process. The pre-fix Standard path mmap'd any + /// basis of 1 MiB or larger, so dereferencing a block past the shrunk EOF + /// faulted an unmapped page and raised `SIGBUS` (exit 135 Linux, 138 Darwin). + /// The windowed `read(2)` reader instead hits EOF early and returns + /// `UnexpectedEof`. Surviving to the assertion (no signal death) is the + /// load-bearing part; the error kind pins the graceful-degradation contract. + #[test] + fn basis_apply_survives_concurrent_truncation() { + use std::num::NonZeroU8; + + let dir = tempdir().expect("tempdir"); + let basis_path = dir.path().join("basis.bin"); + let out_path = dir.path().join("out.bin"); + + // 2 MiB basis: above the legacy 1 MiB mmap threshold, so the pre-fix + // Standard path would have mmap'd it. + const BASIS_LEN: usize = 2 * 1024 * 1024; + let basis_bytes: Vec = (0..BASIS_LEN).map(|i| (i % 251) as u8).collect(); + { + let mut f = File::create(&basis_path).expect("create basis"); + f.write_all(&basis_bytes).expect("write basis"); + f.sync_all().ok(); + } + + // Signature over the full, untruncated basis - the block layout the + // applicator will index against. + let layout = + engine::delta::calculate_signature_layout(engine::delta::SignatureLayoutParams::new( + BASIS_LEN as u64, + None, + protocol::ProtocolVersion::NEWEST, + NonZeroU8::new(16).unwrap(), + )) + .expect("layout"); + let signature = engine::signature::generate_file_signature( + File::open(&basis_path).expect("reopen basis"), + layout, + engine::signature::SignatureAlgorithm::Md4, + ) + .expect("signature"); + let last_block = (signature.layout().block_count() as usize) + .checked_sub(1) + .expect("basis has at least one block"); + + let out = File::create(&out_path).expect("create out"); + let config = DeltaApplyConfig { + sparse: false, + writer_kind: BasisWriterKind::Standard, + cow_policy: fast_io::CowPolicy::Auto, + }; + let verifier = ChecksumVerifier::for_algorithm(protocol::ChecksumAlgorithm::MD5); + let mut applicator = DeltaApplicator::new( + out, + &config, + verifier, + Some(&signature), + Some(basis_path.as_path()), + ) + .expect("construct applicator"); + assert!( + !applicator.basis_uses_mmap(), + "basis must not be mmap-backed" + ); + + // Another process truncates the basis to a few KiB while we hold it + // open. The last block now lies far past the new EOF. + File::options() + .write(true) + .open(&basis_path) + .expect("reopen for truncation") + .set_len(8 * 1024) + .expect("truncate basis"); + + // Referencing the now-missing tail block must return an error, not + // crash the process with a bus error. + let err = applicator + .apply_block_ref(last_block) + .expect_err("block past truncated EOF must return Err, not SIGBUS"); + assert_eq!( + err.kind(), + io::ErrorKind::UnexpectedEof, + "short read past the shrunk basis must surface as UnexpectedEof, got {err:?}", + ); + } + #[test] fn should_try_kernel_copy_requires_noop_verifier() { // CSUM_NONE -> fast path eligible. diff --git a/crates/transfer/src/receiver/basis.rs b/crates/transfer/src/receiver/basis.rs index 4cb31109a..0447a82b8 100644 --- a/crates/transfer/src/receiver/basis.rs +++ b/crates/transfer/src/receiver/basis.rs @@ -18,6 +18,7 @@ use engine::signature::{ use protocol::ProtocolVersion; use crate::config::ReferenceDirectory; +use crate::constants::MAX_MAP_SIZE; /// Result of searching for a basis file via [`find_basis_file_with_config`]. /// @@ -433,52 +434,27 @@ fn generate_basis_signature( let parallel = parallel_checksum_enabled(); - // Large regular baseses read the whole file block-by-block to hash it. A - // raw `File` costs one read syscall per block (default block ~700 bytes), - // so a multi-GB basis issues millions of syscalls. Memory-mapping the basis - // replaces those syscalls with demand-paged access while reading the exact - // same bytes in the same order - the per-block rolling and strong sums, and - // therefore the wire signature and the resulting delta, are byte-identical. - // Small baseses stay on the buffered path (mmap setup is not worth it) and - // any mmap failure (NFS/FUSE/procfs, ENOMEM, non-regular file) falls back - // to the raw `File` reader. - #[cfg(unix)] - let signature = if basis_size >= MMAP_BASIS_THRESHOLD_BYTES { - match fast_io::MmapReader::from_file(basis_file) { - Ok(mapped) => { - let _ = mapped.advise_sequential(); - compute_basis_signature( - mapped, - basis_size, - layout, - config.checksum_algorithm, - parallel, - ) - } - Err(_) => match reopen_basis(&basis_path) { - Some(file) => compute_basis_signature( - file, - basis_size, - layout, - config.checksum_algorithm, - parallel, - ), - None => return BasisFileResult::EMPTY, - }, - } - } else { - compute_basis_signature( - basis_file, - basis_size, - layout, - config.checksum_algorithm, - parallel, - ) - }; - - #[cfg(not(unix))] + // Hash the basis block-by-block through a sliding read() window, never by + // memory-mapping it. A large basis read one block at a time from a raw + // `File` costs one read syscall per block (default block ~700 bytes), so a + // multi-GB basis would issue millions of syscalls; a large buffered reader + // amortises that to one read() per `MAX_MAP_SIZE` window while touching the + // exact same bytes in the same order, so the per-block rolling and strong + // sums - and therefore the wire signature and resulting delta - are + // byte-identical to any other read path. + // + // The basis must never be mmap'd here: if another process truncates the + // file mid-read (upstream's canonical example is a mailer rewriting the + // file), dereferencing the now-unmapped tail raises SIGBUS while the kernel + // faults the page in on our behalf, killing the process with no signal-safe + // recovery. A read(2) past the shrunk EOF instead returns short, which the + // signature generator reports as an I/O error; the caller then drops the + // basis and falls back to a whole-file transfer. + // upstream: fileio.c:214-217 comment + map_ptr() deliberately use read(2) + // instead of mmap(2) on basis files for exactly this reason. + let reader = std::io::BufReader::with_capacity(MAX_MAP_SIZE, basis_file); let signature = compute_basis_signature( - basis_file, + reader, basis_size, layout, config.checksum_algorithm, @@ -496,32 +472,6 @@ fn generate_basis_signature( } } -/// Minimum basis-file size at which the receiver memory-maps the basis for -/// signature hashing instead of reading it through a raw `File`. -/// -/// Matches `fast_io::MmapReader`'s own `MMAP_THRESHOLD` (64 KiB). Below this the -/// buffered read wins because mmap setup and page-fault overhead outweigh the -/// saved syscalls; above it the syscall-per-block cost dominates. The choice is -/// a pure local performance knob: the computed signature is byte-identical -/// either way, so it is never negotiated or sent over the wire. -#[cfg(unix)] -const MMAP_BASIS_THRESHOLD_BYTES: u64 = 64 * 1024; - -/// Re-opens a basis file through the same hardened, symlink-refusing open used -/// for the original lookup, so the mmap fallback path never re-introduces a -/// symlinked-basename window. Returns `None` if the file can no longer be -/// opened as a regular file, in which case the caller treats the basis as -/// absent and falls back to whole-file transfer. -#[cfg(unix)] -fn reopen_basis(path: &std::path::Path) -> Option { - let file = fast_io::open_basis_nofollow(path).ok()?; - if file.metadata().ok()?.is_file() { - Some(file) - } else { - None - } -} - /// Process-wide policy set by the `--checksum-threads` CLI flag, overriding the /// bench-validated default (parallel-on above the threshold). /// @@ -899,19 +849,20 @@ mod tests { ); } - /// Byte-transparency gate for the default-mmap basis read: the signature - /// computed by memory-mapping a large basis (`MmapReader::from_file`) MUST - /// be byte-identical to the signature computed by reading the same basis - /// through a raw `File`. The signature is what the sender matches against, - /// so any divergence between the two read paths would change the delta and - /// break interop. Size exceeds `MMAP_BASIS_THRESHOLD_BYTES` so the mmap - /// branch is exercised. + /// Byte-transparency gate for the windowed basis read: the signature + /// computed by streaming a large basis through the production + /// `BufReader`-over-`File` reader MUST be byte-identical to the signature + /// computed by reading the same basis straight through a raw `File`. The + /// signature is what the sender matches against, so any divergence between + /// the two read paths would change the delta and break interop. Size spans + /// several `MAX_MAP_SIZE` windows so the sliding read is exercised. #[cfg(unix)] #[test] - fn mmap_basis_signature_equals_buffered_file_signature() { - use std::io::Write; + fn windowed_basis_signature_equals_raw_file_signature() { + use std::io::{BufReader, Write}; - let size = (MMAP_BASIS_THRESHOLD_BYTES + 4096) as usize; + // Multiple 256 KiB windows so the buffered reader refills repeatedly. + let size = MAX_MAP_SIZE * 3 + 4096; // Non-trivial, non-repeating bytes so blocks hash distinctly and a // mis-read (wrong offset/length) would surface as a signature diff. let data: Vec = (0..size).map(|i| ((i * 31 + 7) % 251) as u8).collect(); @@ -932,13 +883,18 @@ mod tests { )) .expect("layout"); - let mapped = fast_io::MmapReader::from_file( + let windowed = BufReader::with_capacity( + MAX_MAP_SIZE, fast_io::open_basis_nofollow(&path).expect("open basis"), + ); + let via_windowed = compute_basis_signature( + windowed, + size as u64, + layout, + SignatureAlgorithm::Md4, + false, ) - .expect("mmap basis"); - let via_mmap = - compute_basis_signature(mapped, size as u64, layout, SignatureAlgorithm::Md4, false) - .expect("mmap signature"); + .expect("windowed signature"); let file = fast_io::open_basis_nofollow(&path).expect("open basis"); let via_file = @@ -946,40 +902,41 @@ mod tests { .expect("file signature"); assert_eq!( - via_mmap, via_file, - "mmap basis signature diverged from buffered-file signature", + via_windowed, via_file, + "windowed basis signature diverged from raw-file signature", ); - // Also assert against the parallel path so the mmap read composes with - // both signature generators used in production. - let mapped_par = fast_io::MmapReader::from_file( + // Also assert against the parallel path so the windowed read composes + // with both signature generators used in production. + let windowed_par = BufReader::with_capacity( + MAX_MAP_SIZE, fast_io::open_basis_nofollow(&path).expect("open basis"), - ) - .expect("mmap basis"); - let via_mmap_parallel = compute_basis_signature( - mapped_par, + ); + let via_windowed_parallel = compute_basis_signature( + windowed_par, size as u64, layout, SignatureAlgorithm::Md4, true, ) - .expect("mmap parallel signature"); + .expect("windowed parallel signature"); assert_eq!( - via_mmap_parallel, via_file, - "mmap parallel basis signature diverged from buffered-file signature", + via_windowed_parallel, via_file, + "windowed parallel basis signature diverged from raw-file signature", ); } - /// End-to-end check that `generate_basis_signature` (the production entry - /// that now defaults to mmap for large baseses) yields the same signature - /// the raw-`File` path produces. Guards against the dispatch wrapper picking - /// a divergent branch. + /// End-to-end check that `generate_basis_signature` (the production entry, + /// which streams the basis through the windowed `BufReader` reader) yields + /// the same signature the raw-`File` path produces. Guards against the + /// dispatch wrapper picking a divergent branch. #[cfg(unix)] #[test] - fn generate_basis_signature_mmap_default_matches_raw_file() { + fn generate_basis_signature_matches_raw_file() { use std::io::Write; - let size = (MMAP_BASIS_THRESHOLD_BYTES + 1234) as usize; + // Span several windows so the sliding read is genuinely exercised. + let size = MAX_MAP_SIZE * 2 + 1234; let data: Vec = (0..size).map(|i| ((i * 17 + 3) % 251) as u8).collect(); let tmp = tempfile::tempdir().expect("tempdir"); @@ -997,7 +954,7 @@ mod tests { compat_flags: None, }; - // Production path (mmap default engaged because size >= threshold). + // Production path (windowed BufReader over the basis file). let via_default = generate_basis_signature( fast_io::open_basis_nofollow(&path).expect("open basis"), size as u64, @@ -1027,11 +984,70 @@ mod tests { assert_eq!( via_default.signature.as_ref(), Some(&via_raw), - "generate_basis_signature mmap-default diverged from raw-file signature", + "generate_basis_signature windowed read diverged from raw-file signature", ); assert_eq!(via_default.basis_path.as_deref(), Some(path.as_path())); } + /// Regression (#277): `generate_basis_signature` must survive the basis + /// being truncated by another process after it is opened, returning an + /// empty result (whole-file fallback) instead of dying by `SIGBUS`. The + /// pre-fix path mmap'd baseses >= 64 KiB, so hashing a block past the + /// shrunk EOF faulted an unmapped page and killed the process (exit 135 on + /// Linux, 138 on Darwin). The windowed `read(2)` reader hits EOF early, the + /// signature generator returns an error, and the basis is dropped. The + /// layout is derived from the ORIGINAL size, mirroring the live path where + /// the generator sizes the layout from the pre-truncation stat. + #[cfg(unix)] + #[test] + fn generate_basis_signature_survives_concurrent_truncation() { + use std::io::Write; + + // 1 MiB basis: well above the pre-fix 64 KiB mmap threshold. + const BASIS_LEN: usize = 1024 * 1024; + let data: Vec = (0..BASIS_LEN).map(|i| (i % 251) as u8).collect(); + + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("basis.bin"); + { + let mut f = fs::File::create(&path).expect("create basis"); + f.write_all(&data).expect("write basis"); + f.sync_all().ok(); + } + + let cfg = SignatureGenerationConfig { + protocol: ProtocolVersion::NEWEST, + checksum_length: NonZeroU8::new(16).unwrap(), + checksum_algorithm: SignatureAlgorithm::Md4, + compat_flags: None, + }; + + // Open the basis exactly as the live path does, then let "another + // process" truncate it before the bytes are hashed. + let basis_file = fast_io::open_basis_nofollow(&path).expect("open basis"); + fs::OpenOptions::new() + .write(true) + .open(&path) + .expect("reopen for truncation") + .set_len(8 * 1024) + .expect("truncate basis"); + + // Must not SIGBUS. Surviving to inspect the result is the assertion; + // the shrunk basis yields the empty (whole-file fallback) result. + let result = generate_basis_signature( + basis_file, + BASIS_LEN as u64, + path.clone(), + protocol::FnameCmpType::Fname, + None, + cfg, + ); + assert!( + result.signature.is_none(), + "a basis truncated mid-read must fall back to whole-file transfer", + ); + } + /// Issue #264 regression: when the destination is absent but an /// interrupted transfer left a same-named file under `--partial-dir`, the /// generator must select that partial file as the delta basis and tag it