diff --git a/Cargo.lock b/Cargo.lock index bb6b3b451..4e1388f36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3057,7 +3057,6 @@ dependencies = [ "image", "inferno", "libc", - "memmap2", "napi", "napi-build", "napi-derive", diff --git a/Cargo.toml b/Cargo.toml index 326d3b7b9..e3b09b66d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -237,10 +237,6 @@ smallvec = { version = "1.15.1", features = [ # ────────────────────────────────────────────────────────────────────────────── xxhash-rust = { version = "0.8", features = ["xxh64"] } -# ────────────────────────────────────────────────────────────────────────────── -# Memory Mapping -# ────────────────────────────────────────────────────────────────────────────── -memmap2 = "0.9" # ────────────────────────────────────────────────────────────────────────────── # System & Platform diff --git a/crates/pi-natives/Cargo.toml b/crates/pi-natives/Cargo.toml index 4207369d3..f3d02430e 100644 --- a/crates/pi-natives/Cargo.toml +++ b/crates/pi-natives/Cargo.toml @@ -29,7 +29,6 @@ icy_sixel.workspace = true ignore.workspace = true image = { workspace = true, features = ["bmp"] } inferno.workspace = true -memmap2.workspace = true napi.workspace = true napi-derive.workspace = true parking_lot.workspace = true diff --git a/crates/pi-natives/src/clipboard.rs b/crates/pi-natives/src/clipboard.rs index c7b2251b2..fdf862c1e 100644 --- a/crates/pi-natives/src/clipboard.rs +++ b/crates/pi-natives/src/clipboard.rs @@ -273,7 +273,7 @@ mod tests { d } - /// `CF_DIBV5` as PixPin (Qt) places it, after arboard's + /// `CF_DIBV5` as `PixPin` (Qt) places it, after arboard's /// `maybe_tweak_header` rewrite: a 124-byte `BITMAPV5HEADER` carrying /// `BI_BITFIELDS` compression with the BGRA masks embedded in the header /// and pixels immediately after it. This is the exact buffer shape that diff --git a/crates/pi-natives/src/grep.rs b/crates/pi-natives/src/grep.rs index 9e6fbce25..e40c45c52 100644 --- a/crates/pi-natives/src/grep.rs +++ b/crates/pi-natives/src/grep.rs @@ -35,7 +35,6 @@ use smallvec::SmallVec; use crate::{glob_util, iofs, task}; const MAX_FILE_BYTES: u64 = 4 * 1024 * 1024; -const SMALL_FILE_READ_BYTES: u64 = 128 * 1024; /// Output mode for [`search`] and [`grep`] (string values match JS callers). #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -265,10 +264,8 @@ struct FileSearchResult { limit_reached: bool, } -enum FileBytes { - Mapped(memmap2::Mmap), - Owned(Vec), -} +/// Owned bytes captured from a file before search. +type FileBytes = Vec; /// Outcome of attempting to read a file for searching. enum ReadFile { @@ -280,15 +277,6 @@ enum ReadFile { Skipped, } -impl FileBytes { - fn as_slice(&self) -> &[u8] { - match self { - Self::Mapped(mapped) => mapped.as_ref(), - Self::Owned(bytes) => bytes.as_slice(), - } - } -} - impl MatchCollector { fn new( max_count: Option, @@ -661,10 +649,19 @@ fn build_searcher( .build() } +const FILE_CLASSIFICATION_READ_BYTES: u64 = MAX_FILE_BYTES + 1; + fn file_len_exceeds_limit(len: usize) -> bool { u64::try_from(len).map_or(true, |len| len > MAX_FILE_BYTES) } +fn read_owned_prefix(mut file: File, limit: u64) -> io::Result> { + let mut buffer = + Vec::with_capacity(usize::try_from(limit).expect("bounded read limit fits usize")); + file.by_ref().take(limit).read_to_end(&mut buffer)?; + Ok(buffer) +} + /// Read file bytes, distinguishing oversized files from other skips. fn read_file_bytes(path: &Path) -> io::Result { read_file_bytes_with_size(path, None) @@ -692,44 +689,13 @@ fn read_file_bytes_with_size(path: &Path, size_hint: Option) -> io::Result< }; if size > MAX_FILE_BYTES { return Ok(ReadFile::Oversized); - } else if size == 0 { - return Ok(ReadFile::Bytes(FileBytes::Owned(Vec::new()))); - } - if size <= SMALL_FILE_READ_BYTES { - let mut buffer = - Vec::with_capacity(usize::try_from(size).expect("bounded small file size fits usize")); - let mut handle = file; - handle.read_to_end(&mut buffer)?; - if file_len_exceeds_limit(buffer.len()) { - return Ok(ReadFile::Oversized); - } - return Ok(ReadFile::Bytes(FileBytes::Owned(buffer))); } - let mapping = unsafe { - // SAFETY: The mapping is read-only and tied to the opened file handle. - // We do not mutate through this view; the map is dropped immediately - // after search for each file. - memmap2::Mmap::map(&file) - }; - - let bytes = if let Ok(mapped) = mapping { - if file_len_exceeds_limit(mapped.len()) { - return Ok(ReadFile::Oversized); - } - FileBytes::Mapped(mapped) - } else { - let mut buffer = - Vec::with_capacity(usize::try_from(size).expect("bounded file size fits usize")); - let mut handle = file; - handle.read_to_end(&mut buffer)?; - if file_len_exceeds_limit(buffer.len()) { - return Ok(ReadFile::Oversized); - } - FileBytes::Owned(buffer) - }; - - Ok(ReadFile::Bytes(bytes)) + let buffer = read_owned_prefix(file, FILE_CLASSIFICATION_READ_BYTES)?; + if file_len_exceeds_limit(buffer.len()) { + return Ok(ReadFile::Oversized); + } + Ok(ReadFile::Bytes(buffer)) } // --------------------------------------------------------------------------- @@ -1218,12 +1184,11 @@ struct PassState { skipped_oversized: AtomicU64, emitted: AtomicU64, } -/// Memory-map the first [`MAX_FILE_BYTES`] of a file for searching. +/// Read the first [`MAX_FILE_BYTES`] of a file into owned bytes for searching. /// /// Used by the deferred oversized pass: files larger than the cap are searched -/// only over their leading window; the remainder is dropped. mmap-only — never -/// falls back to `read_to_end`, so a multi-gigabyte file never allocates its -/// full contents. Returns [`ReadFile::Skipped`] when the file cannot be mapped. +/// only over their leading window; the remainder is dropped. The bounded owned +/// read avoids mmap page faults when the backing file is rewritten concurrently. fn read_file_prefix(path: &Path) -> io::Result { let file = match File::open(path) { Ok(file) => file, @@ -1240,17 +1205,11 @@ fn read_file_prefix(path: &Path) -> io::Result { } let len = metadata.len(); if len == 0 { - return Ok(ReadFile::Bytes(FileBytes::Owned(Vec::new()))); - } - let window = - usize::try_from(len.min(MAX_FILE_BYTES)).expect("window is bounded by MAX_FILE_BYTES"); - // SAFETY: read-only mapping tied to the open handle, bounded to `window` - // bytes (<= file length). Dropped immediately after the per-file search. - let mapping = unsafe { memmap2::MmapOptions::new().len(window).map(&file) }; - match mapping { - Ok(mapped) => Ok(ReadFile::Bytes(FileBytes::Mapped(mapped))), - Err(_) => Ok(ReadFile::Skipped), + return Ok(ReadFile::Bytes(Vec::new())); } + let window = len.min(MAX_FILE_BYTES); + let buffer = read_owned_prefix(file, window)?; + Ok(ReadFile::Bytes(buffer)) } /// Read one candidate per `policy` and search it, classifying the result. @@ -3188,6 +3147,30 @@ mod tests { assert_eq!(result.matches[0].path, "big.txt"); } + #[cfg(unix)] + #[test] + fn oversized_prefix_read_returns_stable_snapshot_after_rewrite() { + let root = TempDirGuard::new(); + let path = root.path().join("big.txt"); + let prefix_len = usize::try_from(super::MAX_FILE_BYTES).expect("MAX_FILE_BYTES fits usize"); + let oversized_len = prefix_len + 1024; + fs::write(&path, vec![b'a'; oversized_len]).expect("write original oversized file"); + + let captured = match super::read_file_prefix(&path).expect("read oversized prefix") { + super::ReadFile::Bytes(bytes) => bytes, + super::ReadFile::Oversized => panic!("prefix reader should return the bounded prefix"), + super::ReadFile::Skipped => panic!("prefix reader should read a regular oversized file"), + }; + assert_eq!(captured.as_slice().len(), prefix_len); + + fs::write(&path, vec![b'b'; oversized_len]).expect("rewrite backing file"); + + assert!( + captured.as_slice().iter().all(|&byte| byte == b'a'), + "captured prefix must remain the original bytes after the backing file is rewritten", + ); + } + #[cfg(unix)] #[test] fn oversized_results_follow_normal_results_regardless_of_path_order() { diff --git a/crates/pi-natives/src/shell.rs b/crates/pi-natives/src/shell.rs index e159ba6ed..88ff17db5 100644 --- a/crates/pi-natives/src/shell.rs +++ b/crates/pi-natives/src/shell.rs @@ -399,7 +399,7 @@ mod tests { /// the pre-fix bridge (`flume::unbounded` + fire-and-forget /// `ThreadsafeFunctionCallMode::NonBlocking`) the same harness accumulates /// the producer's entire surplus in the queue (measured: a 32 MiB stream - /// queued all 33_554_432 bytes while the consumer stalled). + /// queued all `33_554_432` bytes while the consumer stalled). #[tokio::test(flavor = "multi_thread")] async fn bridge_pump_bounds_queue_and_delivers_all_bytes() { const CHUNKS: usize = 512;