project: Don't let a canceled caller leak a loading worktree forever (#61009)

# Objective

`WorktreeStore::find_or_create_worktree` inserts the shared
worktree-creation task into `loading_worktrees` and relies on the task
it returns to each caller to remove that entry once creation resolves.
But the creation task keeps running through the clone the map itself
holds, while the map cleanup lives only in the callers' returned tasks.

If every caller is cancelled before creation resolves, the resolved task
stays in `loading_worktrees` forever, retaining the `Entity<Worktree>`
captured in its result (a `Shared` task memoizes its output). Such a
worktree can never be released: `remove_worktree` only unlists it, so
its background scan keeps running and its snapshot keeps growing for the
lifetime of the window. The stale entry also keeps
`initial_scan_complete` permanently `false` (that flag is
`loading_worktrees.is_empty() && …`).

Callers are cancelled routinely — worktree creation is async and can
take seconds on a large tree, while the tasks awaiting it are owned by
UI that the user can close at any time (a tab or pane, a debugger panel
resolving a path, an agent session, or the whole window). See the
existing note in `crates/zed/src/zed.rs` that external-file worktrees
are "released on file close".

Observed in the wild: a home-directory worktree removed from the project
kept scanning for hours and grew Zed past 45 GB; neither removing the
folder nor ending the agent session freed it — only quitting Zed. (The
scan-amplification half of that incident is #60988.)

## Solution

Spawn the map cleanup as its own detached task, next to the map
insertion, so a loading entry always leaves `loading_worktrees` when
loading resolves regardless of what happens to the callers. The returned
per-caller task is unchanged apart from no longer owning that cleanup.

## Testing

- Added `test_worktree_released_when_creation_caller_is_cancelled`: it
requests a worktree, drops the returned task immediately (as a cancelled
caller would), lets creation complete, removes the worktree, and asserts
the entity is released. It fails on `main` and passes with this change.
- Full worktree-related project integration suite is green (45/45).

## 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
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed a memory leak where a worktree whose creation was requested by a
since-cancelled task (e.g. a folder opened as its owning
tab/panel/window closed) could never be released, leaving its background
scan running and its snapshot growing for the lifetime of the window.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
This commit is contained in:
Toru NayukiandKirill Bulatov authored and GitHub committed 2026-08-12 18:13:37 +00:00
1 parent 63d4e29a12
commit fa19fe3c5c
6 files changed
+304 -27

No files matched your search

+5
View File
@@ -2180,6 +2180,7 @@ mod tests {
"This is file2.rs".as_bytes().to_vec(),
)
.await;
cx.run_until_parked();
// Base document with {ABS} placeholder for absolute path prefix.
// Each test case replaces a specific line to add cursor (ˇ) or highlight («»ˇ) markers.
@@ -2349,6 +2350,7 @@ Sentence ending file2.rs.
.to_vec(),
)
.await;
cx.run_until_parked();
// file2.rs:5:3 should be highlighted and clickable
cx.set_state(indoc! {"
@@ -2425,6 +2427,7 @@ Sentence ending file2.rs.
.to_vec(),
)
.await;
cx.run_until_parked();
// file2.rs:3 should be highlighted and clickable
cx.set_state(indoc! {"
@@ -2482,6 +2485,7 @@ Sentence ending file2.rs.
"line 1\nline 2\nline 3\n".as_bytes().to_vec(),
)
.await;
cx.run_until_parked();
// file2.rs:2:in should resolve to file2.rs line 2 (like Ruby backtraces)
cx.set_state(indoc! {"
@@ -2540,6 +2544,7 @@ Sentence ending file2.rs.
.to_vec(),
)
.await;
cx.run_until_parked();
// Markdown link [text](file2.rs:3:2) should highlight only the inner link,
// not the surrounding markdown syntax.
+8 -1
View File
@@ -5399,7 +5399,7 @@ mod tests {
let commits = generate_random_commit_dag(&mut rng, 10, false);
fs.set_graph_commits(Path::new("/project/.git"), commits.clone());
let project = Project::test(fs.clone(), [Path::new("/project")], cx).await;
let project = Project::test(fs.clone(), [], cx).await;
let observed_repository_events = Arc::new(Mutex::new(Vec::new()));
project.update(cx, |project, cx| {
let observed_repository_events = observed_repository_events.clone();
@@ -5413,6 +5413,13 @@ mod tests {
})
.detach();
});
project
.update(cx, |project, cx| {
project.create_worktree("/project", true, cx)
})
.await
.unwrap();
cx.run_until_parked();
let repository = project.read_with(cx, |project, cx| {
project
+27 -24
View File
@@ -797,27 +797,21 @@ impl WorktreeStore {
}
};
self.loading_worktrees
.insert(abs_path.clone(), task.shared());
let loading_task = cx.spawn({
let abs_path = abs_path.clone();
async move |this, cx| {
let result = task.await;
this.update(cx, |this, cx| {
this.loading_worktrees.remove(&abs_path);
if !visible || !this.scanning_enabled || result.is_err() {
this.update_initial_scan_state(cx);
}
})
.ok();
if visible && self.scanning_enabled {
*self.initial_scan_complete.0.borrow_mut() = false;
}
}
let task = self.loading_worktrees.get(&abs_path).unwrap().clone();
cx.spawn(async move |this, cx| {
let result = task.await;
this.update(cx, |this, cx| {
this.loading_worktrees.remove(&abs_path);
if !visible || !this.scanning_enabled || result.is_err() {
this.update_initial_scan_state(cx);
}
})
.ok();
match result {
Ok(worktree) => {
if !is_via_collab {
if let Ok(worktree) = &result
&& !is_via_collab
{
if let Some((trusted_worktrees, worktree_store)) = this
.update(cx, |_, cx| {
TrustedWorktrees::try_get_global(cx).zip(Some(cx.entity()))
@@ -836,16 +830,25 @@ impl WorktreeStore {
this.update(cx, |this, cx| {
if this.scanning_enabled && visible {
this.observe_worktree_scan_completion(&worktree, cx);
this.observe_worktree_scan_completion(worktree, cx);
}
})
.ok();
}
Ok(worktree)
result
}
Err(err) => Err((*err).cloned()),
});
self.loading_worktrees
.insert(abs_path.clone(), loading_task.shared());
if visible && self.scanning_enabled {
*self.initial_scan_complete.0.borrow_mut() = false;
}
})
}
let task = self.loading_worktrees.get(&abs_path).unwrap().clone();
cx.background_spawn(async move { task.await.map_err(|error| (*error).cloned()) })
}
fn create_remote_worktree(
@@ -16252,6 +16252,81 @@ mod disable_ai_settings_tests {
}
}
#[gpui::test]
async fn test_worktree_released_when_creation_caller_is_cancelled(cx: &mut gpui::TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(path!("/project"), serde_json::json!({ "a.rs": "" }))
.await;
fs.insert_tree(path!("/other"), serde_json::json!({ "b.rs": "" }))
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
cx.run_until_parked();
// Drop the returned task immediately, as a cancelled caller would; creation
// still completes through the clone held by `loading_worktrees`.
let task = project.update(cx, |project, cx| {
project.find_or_create_worktree(path!("/other"), true, cx)
});
drop(task);
cx.run_until_parked();
let created = project.read_with(cx, |project, cx| {
project
.worktrees(cx)
.find(|worktree| worktree.read(cx).abs_path().ends_with("other"))
.map(|worktree| (worktree.read(cx).id(), worktree.downgrade()))
});
let Some((worktree_id, weak_worktree)) = created else {
panic!("worktree creation should complete even when the caller is cancelled");
};
project.update(cx, |project, cx| project.remove_worktree(worktree_id, cx));
cx.run_until_parked();
assert!(
weak_worktree.upgrade().is_none(),
"removed worktree should be released even when its creation task's caller was cancelled"
);
}
#[gpui::test]
async fn test_initial_scan_completes_when_creation_caller_is_cancelled(
cx: &mut gpui::TestAppContext,
) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(path!("/project"), serde_json::json!({ "a.rs": "" }))
.await;
fs.insert_tree(path!("/other"), serde_json::json!({ "b.rs": "" }))
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
cx.run_until_parked();
let task = project.update(cx, |project, cx| {
project.find_or_create_worktree(path!("/other"), true, cx)
});
drop(task);
cx.run_until_parked();
let (initial_scan_completed, loading_worktrees) = project.read_with(cx, |project, cx| {
let worktree_store = project.worktree_store().read(cx);
(
worktree_store.initial_scan_completed(),
worktree_store.diagnostics(cx).loading_worktrees,
)
});
assert_eq!(
loading_worktrees, 0,
"no loading entry should remain after creation resolves"
);
assert!(
initial_scan_completed,
"initial scan should complete even when the creation task's caller was cancelled"
);
}
/// Regression test for https://github.com/zed-industries/zed/issues/60424.
///
/// The committed text contains repeated `end\n\n` line runs, so the deletion
+55 -2
View File
@@ -288,11 +288,43 @@ struct BackgroundScannerState {
/// inode-based rename heuristics in that case.
#[derive(Default)]
struct RemovedEntries {
current: RemovedEntriesGeneration,
previous: RemovedEntriesGeneration,
}
#[derive(Default)]
struct RemovedEntriesGeneration {
by_inode: HashMap<u64, Entry>,
by_path: HashMap<Arc<RelPath>, Entry>,
}
impl RemovedEntries {
fn insert(&mut self, entry: &Entry) {
self.current.insert(entry);
}
fn take_by_path(&mut self, path: &RelPath, inode: u64) -> Option<Entry> {
self.current
.take_by_path(path, inode)
.or_else(|| self.previous.take_by_path(path, inode))
}
fn take_by_inode(&mut self, inode: u64) -> Option<Entry> {
self.current
.take_by_inode(inode)
.or_else(|| self.previous.take_by_inode(inode))
}
fn rotate(&mut self) -> impl Iterator<Item = Entry> {
let dropped = mem::replace(&mut self.previous, mem::take(&mut self.current));
dropped
.by_inode
.into_values()
.chain(dropped.by_path.into_values())
}
}
impl RemovedEntriesGeneration {
fn insert(&mut self, entry: &Entry) {
self.by_path.insert(entry.path.clone(), entry.clone());
match self.by_inode.entry(entry.inode) {
@@ -4577,7 +4609,28 @@ impl BackgroundScanner {
async fn process_scan_request(&self, mut request: ScanRequest, scanning: bool) -> bool {
log::debug!("rescanning paths {:?}", request.relative_paths);
{
let state = self.state.lock().await;
let mut missing_ancestors = Vec::new();
for path in &request.relative_paths {
let mut candidates = Vec::new();
let mut covered_by_existing_ancestor = false;
for ancestor in path.ancestors().skip(1) {
if let Some(entry) = state.snapshot.entry_for_path(ancestor) {
covered_by_existing_ancestor =
entry.kind == EntryKind::UnloadedDir || entry.kind == EntryKind::File;
break;
}
candidates.push(ancestor.into_arc());
}
if !covered_by_existing_ancestor {
missing_ancestors.extend(candidates);
}
}
request.relative_paths.extend(missing_ancestors);
}
request.relative_paths.sort_unstable();
request.relative_paths.dedup();
self.forcibly_load_paths(&request.relative_paths).await;
let root_path = self.state.lock().await.snapshot.abs_path.clone();
@@ -5099,8 +5152,8 @@ impl BackgroundScanner {
{
let mut state = self.state.lock().await;
state.snapshot.completed_scan_id = state.snapshot.scan_id;
let RemovedEntries { by_inode, by_path } = mem::take(&mut state.removed_entries);
for entry in by_inode.into_values().chain(by_path.into_values()) {
let dropped_entries = state.removed_entries.rotate().collect::<Vec<_>>();
for entry in dropped_entries {
state.scanned_dirs.remove(&entry.id);
}
}
@@ -87,6 +87,140 @@ async fn test_traversal(cx: &mut TestAppContext) {
})
}
#[gpui::test]
async fn test_entry_id_is_reused_when_rename_events_are_split(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.background_executor.clone());
fs.insert_tree(
path!("/root"),
json!({
"one.rs": "const ONE: usize = 1;",
}),
)
.await;
let tree = Worktree::local(
Path::new(path!("/root")),
true,
fs.clone(),
Default::default(),
true,
WorktreeId::from_proto(0),
&mut cx.to_async(),
)
.await
.unwrap();
cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
.await;
let old_entry_id = tree.read_with(cx, |tree, _| {
tree.entry_for_path(rel_path("one.rs")).unwrap().id
});
fs.pause_events();
fs.rename(
Path::new(path!("/root/one.rs")),
Path::new(path!("/root/three.rs")),
Default::default(),
)
.await
.unwrap();
assert_eq!(fs.buffered_event_count(), 2);
fs.flush_events(1);
cx.executor().run_until_parked();
fs.flush_events(1);
cx.executor().run_until_parked();
tree.read_with(cx, |tree, _| {
assert_eq!(tree.entry_for_path(rel_path("one.rs")), None);
assert_eq!(
tree.entry_for_path(rel_path("three.rs")).unwrap().id,
old_entry_id,
"a rename whose removal and creation events arrive in separate batches must reuse the entry id"
);
});
}
#[gpui::test]
async fn test_rescan_requests_processed_before_root_creation_events(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.background_executor.clone());
let tree = Worktree::local(
Path::new(path!("/root")),
true,
fs.clone(),
Default::default(),
true,
WorktreeId::from_proto(0),
&mut cx.to_async(),
)
.await
.unwrap();
cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
.await;
fs.pause_events();
fs.insert_tree(
path!("/root"),
json!({
"a": {
"b": { "f1": "" },
"c": { "f2": "" },
}
}),
)
.await;
let mut refresh = tree.update(cx, |tree, _| {
tree.as_local()
.unwrap()
.refresh_entries_for_paths(vec![rel_path("a/b/f1").into()])
});
refresh.recv().await;
let mut refresh = tree.update(cx, |tree, _| {
tree.as_local()
.unwrap()
.refresh_entries_for_paths(vec![rel_path("a/c/f2").into()])
});
refresh.recv().await;
tree.read_with(cx, |tree, _| {
assert_eq!(
tree.entries(true, 0)
.map(|entry| entry.path.as_ref())
.collect::<Vec<_>>(),
vec![
rel_path(""),
rel_path("a"),
rel_path("a/b"),
rel_path("a/b/f1"),
rel_path("a/c"),
rel_path("a/c/f2"),
]
);
});
fs.unpause_events_and_flush();
cx.executor().run_until_parked();
tree.read_with(cx, |tree, _| {
assert_eq!(
tree.entries(true, 0)
.map(|entry| entry.path.as_ref())
.collect::<Vec<_>>(),
vec![
rel_path(""),
rel_path("a"),
rel_path("a/b"),
rel_path("a/b/f1"),
rel_path("a/c"),
rel_path("a/c/f2"),
]
);
});
}
#[gpui::test(iterations = 10)]
async fn test_circular_symlinks(cx: &mut TestAppContext) {
init_test(cx);