fs: Fix crash loop from closedir panic during worktree scan (#59953)
# 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 <cole@zed.dev>
This commit is contained in:
1 parent
b73d91382d
commit
ef0b6f158d
4 files changed
+66
-12
No files matched your search
Generated
+1
@@ -6917,6 +6917,7 @@ dependencies = [
|
||||
"paths",
|
||||
"proto",
|
||||
"rope",
|
||||
"rustix 1.1.2",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"slotmap",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
+60
-12
@@ -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<CString> {
|
||||
})
|
||||
}
|
||||
|
||||
// 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<impl Send + Iterator<Item = Result<PathBuf>>> {
|
||||
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<impl Send + Iterator<Item = Result<PathBuf>>> {
|
||||
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<Pin<Box<dyn Send + Stream<Item = Result<PathBuf>>>>> {
|
||||
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:?}")
|
||||
))
|
||||
|
||||
Reference in new issue
Block a user