From ef0b6f158dedbd8bbe264d6fc80a7204cadd05e6 Mon Sep 17 00:00:00 2001 From: MB Date: Sun, 26 Jul 2026 23:13:39 +0200 Subject: [PATCH] fs: Fix crash loop from closedir panic during worktree scan (#59953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Fixes #59952. Zed crash-loops on launch. The faulting thread aborts inside the Rust standard library while closing a directory handle during background worktree scanning: `std`'s `DirStream::drop` (`library/std/src/sys/fs/unix.rs`) `assert!`s that `closedir()` succeeds unless the error is `EINTR`. That handle originates in `RealFs::read_dir`, which wraps `std::fs::read_dir(path)` in `stream::iter(...)` and carries the live `ReadDir` across async boundaries; the worktree `BackgroundScanner` (`scan_dir` → `forcibly_load_paths`) later drops the stream. If `closedir()` returns a non-`EINTR` error (e.g. `EBADF` under file-descriptor pressure — seen alongside repeated `unable to start FSEvent stream` warnings on a large dependency tree), `std` panics inside `Drop`. Zed's `crashes::panic_hook` turns any panic into `process::abort()`, and because the same workspace is re-scanned every launch, this is a permanent crash loop. (`catch_unwind` can't help: the hook aborts before unwinding.) ## Solution Stop relying on `std`'s asserting close. Add a `read_dir_entries(path)` helper that reads entries eagerly so the directory handle is opened and closed within a single call: - On unix, read via libc (`opendir`/`readdir`/`closedir`) and **deliberately ignore a failing `closedir`**, so a close error degrades gracefully instead of aborting the process. - On non-unix, keep `std::fs::read_dir` (which has no such assert) but collect eagerly so the handle is dropped within the call. `RealFs::read_dir` now calls this helper. The change is scoped to `crates/fs/src/fs.rs`; the watcher, scanner, and panic hook are untouched. ## Testing - Added unit tests in `crates/fs/src/fs.rs` for `read_dir_entries`: entry listing, `.`/`..` exclusion, empty directories, and the missing-directory error path. - `cargo test -p fs read_dir` → 3 passed. - `RUSTFLAGS="-D warnings" cargo build -p fs`, `cargo clippy -p fs --all-features --all-targets -- -D warnings`, and `cargo fmt -p fs --check` are all clean. - Tested on macOS (aarch64). The `fs` crate compiles within the full `cargo build -p zed` graph. I could not produce a running app binary in my environment (the `gpui_macos` Metal step needs full Xcode, not just Command Line Tools) — reviewers on a full Xcode setup can `cargo run -p zed`. - The original `closedir` `EBADF` is not deterministically reproducible, so the fix is structural rather than verified against a live repro; the tests guard the read path and behavior. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards — N/A (no UI change) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed Zed crash-looping on launch when `closedir` fails during background directory scanning ([#59952](https://github.com/zed-industries/zed/issues/59952)). --------- Co-authored-by: Cole Miller --- Cargo.lock | 1 + Cargo.toml | 1 + crates/fs/Cargo.toml | 4 +++ crates/fs/src/fs.rs | 72 ++++++++++++++++++++++++++++++++++++-------- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d22ca539a..0bf1f8a61c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6917,6 +6917,7 @@ dependencies = [ "paths", "proto", "rope", + "rustix 1.1.2", "serde", "serde_json", "slotmap", diff --git a/Cargo.toml b/Cargo.toml index 3eb608e555..4056f9d568 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -779,6 +779,7 @@ runtimelib = { version = "1.4.0", default-features = false, features = [ ] } rust-embed = { version = "8.11", features = ["include-exclude"] } rustc-hash = "2.1.0" +rustix = { version = "1.1", features = ["fs"] } rustls = { version = "0.23.26" } rustls-platform-verifier = "0.5.0" # WARNING: If you change this, you must also publish a new version of zed-scap to crates.io diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index ef0cb4bd8c..2f1d694f25 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -47,6 +47,10 @@ path.workspace = true is_executable = "1.0.5" notify = "9.0.0-rc.4" trash = { git = "https://github.com/zed-industries/trash-rs", rev = "47761739192828a66b11a94ba5420b82d63c03c5" } + +[target.'cfg(unix)'.dependencies] +rustix.workspace = true + [target.'cfg(target_os = "windows")'.dependencies] windows.workspace = true dunce.workspace = true diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index a7784922c7..9b3adda712 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -9,7 +9,7 @@ use std::sync::atomic::{AtomicU8, AtomicUsize, Ordering}; use std::time::Instant; use util::maybe; -use anyhow::{Context as _, Result, anyhow}; +use anyhow::{Context as _, Result}; use futures::stream::iter; use gpui::App; use gpui::BackgroundExecutor; @@ -629,6 +629,59 @@ fn path_to_c_string(path: &Path) -> io::Result { }) } +// On Unix targets, std::fs::ReadDir panics in its Drop implementation +// when an unexpected error is returned from closedir(2). We hit this +// condition in production; one cause seems to be macOS's FSEventStream +// incorrectly closing fds it doesn't own, resulting in closedir returning +// EBADF, see https://github.com/zed-industries/zed/issues/59952#issuecomment-5080178879. +// +// We also see occasional errors like ENXIO and ETIMEDOUT that seem to +// come from network or other exotic filesystems. +// +// To avoid crashing the app in this situation, we use the rustix analogue of +// ReadDir, which doesn't have this panic in drop. +#[cfg(unix)] +fn read_dir_entries(path: PathBuf) -> Result>> { + use rustix::fs::{Dir, Mode, OFlags}; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let directory_fd = rustix::fs::open( + &path, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC, + Mode::empty(), + ) + .with_context(|| format!("failed to open directory {path:?}"))?; + let directory = + Dir::new(directory_fd).with_context(|| format!("failed to read directory {path:?}"))?; + + Ok(directory.filter_map(move |entry| { + let entry = match entry { + Ok(entry) => entry, + Err(error) => { + return Some(Err(anyhow::Error::new(error) + .context(format!("failed to read directory entry in {path:?}")))); + } + }; + let name = entry.file_name().to_bytes(); + if name == b"." || name == b".." { + return None; + } + Some(Ok(path.join(OsStr::from_bytes(name)))) + })) +} + +#[cfg(not(unix))] +fn read_dir_entries(path: PathBuf) -> Result>> { + let entries = + std::fs::read_dir(&path).with_context(|| format!("failed to open directory {path:?}"))?; + Ok(entries.map(move |entry| { + entry + .map(|entry| entry.path()) + .with_context(|| format!("failed to read directory entry in {path:?}")) + })) +} + #[async_trait::async_trait] impl Fs for RealFs { async fn create_dir(&self, path: &Path) -> Result<()> { @@ -1063,16 +1116,11 @@ impl Fs for RealFs { path: &Path, ) -> Result>>>> { let path = path.to_owned(); - let result = iter( - self.executor - .spawn(async move { std::fs::read_dir(path) }) - .await?, - ) - .map(|entry| match entry { - Ok(entry) => Ok(entry.path()), - Err(error) => Err(anyhow!("failed to read dir entry {error:?}")), - }); - Ok(Box::pin(result)) + let entries = self + .executor + .spawn(async move { read_dir_entries(path) }) + .await?; + Ok(Box::pin(iter(entries))) } async fn watch( @@ -1591,7 +1639,7 @@ impl FakeFsState { Ok(self .try_entry(target, true) .ok_or_else(|| { - anyhow!(io::Error::new( + anyhow::anyhow!(io::Error::new( io::ErrorKind::NotFound, format!("not found: {target:?}") ))