From d63067762a8c4274307979b6a9db7ba89b3c7458 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Tue, 20 Jan 2026 14:58:35 -0600 Subject: [PATCH] Fix config file watch task leak (#47246) Follow-Up-For: #47243 Previously, we would detach tasks spawned to watch config files. However, the task blocked on receiving a file event before checking if the receiver for the updates channel was dropped, causing the task to never exit. The fix here was to return the task explicitly, so that it can be dropped instead of calling `.detach()` on it. There is definitely a way to `select!` between the receiver being dropped and the next file system event, but I couldn't figure it out in a reasonable amount of time and decided it wasn't worth it. Release Notes: - Fixed an issue where a few file descriptors would be leaked each time a project was closed --- crates/agent/src/tests/mod.rs | 3 +- crates/project/src/project_settings.rs | 6 ++- crates/recent_projects/src/remote_servers.rs | 8 ++-- crates/remote_server/src/unix.rs | 6 ++- crates/settings/src/editorconfig_store.rs | 4 +- crates/settings/src/settings_file.rs | 40 ++++++++++---------- crates/zed/src/main.rs | 16 +++++--- crates/zed/src/zed.rs | 38 ++++++++++++++----- 8 files changed, 76 insertions(+), 45 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index e3ff61bb6a39535e32b5596d25235c0dfcd7344e..762b4d9a1393f96116e77d3e265b53633f014e7a 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -3352,11 +3352,12 @@ fn watch_settings(fs: Arc, cx: &mut App) { let fs = fs.clone(); cx.spawn({ async move |cx| { - let mut new_settings_content_rx = settings::watch_config_file( + let (mut new_settings_content_rx, watcher_task) = settings::watch_config_file( cx.background_executor(), fs, paths::settings_file().clone(), ); + let _watcher_task = watcher_task; while let Some(new_settings_content) = new_settings_content_rx.next().await { cx.update(|cx| { diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index d40e5002e9fefe7361b7da51aa08991f7f25f925..493f6da8948ae3e63664ea79d721fbd1417d7748 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -1317,11 +1317,12 @@ impl SettingsObserver { file_path: PathBuf, cx: &mut Context, ) -> Task<()> { - let mut user_tasks_file_rx = + let (mut user_tasks_file_rx, watcher_task) = watch_config_file(cx.background_executor(), fs, file_path.clone()); let user_tasks_content = cx.foreground_executor().block_on(user_tasks_file_rx.next()); let weak_entry = cx.weak_entity(); cx.spawn(async move |settings_observer, cx| { + let _watcher_task = watcher_task; let Ok(task_store) = settings_observer.read_with(cx, |settings_observer, _| { settings_observer.task_store.clone() }) else { @@ -1368,11 +1369,12 @@ impl SettingsObserver { file_path: PathBuf, cx: &mut Context, ) -> Task<()> { - let mut user_tasks_file_rx = + let (mut user_tasks_file_rx, watcher_task) = watch_config_file(cx.background_executor(), fs, file_path.clone()); let user_tasks_content = cx.foreground_executor().block_on(user_tasks_file_rx.next()); let weak_entry = cx.weak_entity(); cx.spawn(async move |settings_observer, cx| { + let _watcher_task = watcher_task; let Ok(task_store) = settings_observer.read_with(cx, |settings_observer, _| { settings_observer.task_store.clone() }) else { diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index 8a0e0189e356b89daea8317f3cd28b1ddcdbbe5a..07fb0f80eb17d0a58acf5e6a59c465680ac3e0e3 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/crates/recent_projects/src/remote_servers.rs @@ -2679,13 +2679,15 @@ impl RemoteServerProjects { } fn spawn_ssh_config_watch(fs: Arc, cx: &Context) -> Task<()> { - let mut user_ssh_config_watcher = + let (mut user_ssh_config_watcher, user_watcher_task) = watch_config_file(cx.background_executor(), fs.clone(), user_ssh_config_file()); - let mut global_ssh_config_watcher = global_ssh_config_file() + let (mut global_ssh_config_watcher, global_watcher_task) = global_ssh_config_file() .map(|it| watch_config_file(cx.background_executor(), fs, it.to_owned())) - .unwrap_or_else(|| futures::channel::mpsc::unbounded().1); + .unwrap_or_else(|| (futures::channel::mpsc::unbounded().1, gpui::Task::ready(()))); cx.spawn(async move |remote_server_projects, cx| { + let _user_watcher_task = user_watcher_task; + let _global_watcher_task = global_watcher_task; let mut global_hosts = BTreeSet::default(); let mut user_hosts = BTreeSet::default(); let mut running_receivers = 2; diff --git a/crates/remote_server/src/unix.rs b/crates/remote_server/src/unix.rs index 90bd00d1e3b02c5e14d6dc5345644e021df19624..5965ad5adbeb2fa8f1108b2dd86ea930b7e3aed1 100644 --- a/crates/remote_server/src/unix.rs +++ b/crates/remote_server/src/unix.rs @@ -941,10 +941,10 @@ fn initialize_settings( fs: Arc, cx: &mut App, ) -> watch::Receiver> { - let user_settings_file_rx = + let (user_settings_file_rx, watcher_task) = watch_config_file(cx.background_executor(), fs, paths::settings_file().clone()); - handle_settings_file_changes(user_settings_file_rx, cx, { + handle_settings_file_changes(user_settings_file_rx, watcher_task, cx, { move |err, _cx| { if let Some(e) = err { log::info!("Server settings failed to change: {}", e); @@ -1007,6 +1007,7 @@ fn initialize_settings( pub fn handle_settings_file_changes( mut server_settings_file: mpsc::UnboundedReceiver, + watcher_task: gpui::Task<()>, cx: &mut App, settings_changed: impl Fn(Option, &mut App) + 'static, ) { @@ -1020,6 +1021,7 @@ pub fn handle_settings_file_changes( .log_err(); }); cx.spawn(async move |cx| { + let _watcher_task = watcher_task; while let Some(server_settings_content) = server_settings_file.next().await { cx.update_global(|store: &mut SettingsStore, cx| { let result = store.set_server_settings(&server_settings_content, cx); diff --git a/crates/settings/src/editorconfig_store.rs b/crates/settings/src/editorconfig_store.rs index d819993b047b14e3a1a0ef17c49a5c7c69ae13f7..fdca88fcf5ca19b1be800940630b856a44d2fad2 100644 --- a/crates/settings/src/editorconfig_store.rs +++ b/crates/settings/src/editorconfig_store.rs @@ -285,9 +285,11 @@ impl EditorconfigStore { cx: &mut Context, ) -> Task<()> { let config_path = dir_path.join(EDITORCONFIG_NAME); - let mut config_rx = watch_config_file(cx.background_executor(), fs, config_path); + let (mut config_rx, watcher_task) = + watch_config_file(cx.background_executor(), fs, config_path); cx.spawn(async move |this, cx| { + let _watcher_task = watcher_task; while let Some(content) = config_rx.next().await { let content = Some(content).filter(|c| !c.is_empty()); let dir_path = dir_path.clone(); diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index 504dd4da241fc246f54e077c461afc5dbcc92ebb..f5d0b973340db70819b2b19ae1352a4e1567d670 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -76,32 +76,30 @@ pub fn watch_config_file( executor: &BackgroundExecutor, fs: Arc, path: PathBuf, -) -> mpsc::UnboundedReceiver { +) -> (mpsc::UnboundedReceiver, gpui::Task<()>) { let (tx, rx) = mpsc::unbounded(); - executor - .spawn(async move { - let (events, _) = fs.watch(&path, Duration::from_millis(100)).await; - futures::pin_mut!(events); + let task = executor.spawn(async move { + let (events, _) = fs.watch(&path, Duration::from_millis(100)).await; + futures::pin_mut!(events); - let contents = fs.load(&path).await.unwrap_or_default(); - if tx.unbounded_send(contents).is_err() { - return; - } + let contents = fs.load(&path).await.unwrap_or_default(); + if tx.unbounded_send(contents).is_err() { + return; + } - loop { - if events.next().await.is_none() { - break; - } + loop { + if events.next().await.is_none() { + break; + } - if let Ok(contents) = fs.load(&path).await - && tx.unbounded_send(contents).is_err() - { - break; - } + if let Ok(contents) = fs.load(&path).await + && tx.unbounded_send(contents).is_err() + { + break; } - }) - .detach(); - rx + } + }); + (rx, task) } pub fn watch_config_dir( diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3e54c3f10baf73dd813720b462c5bc9c9eb52897..62f585ec1a7216a79df51b7ac75a155c5dbaefdb 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -362,17 +362,17 @@ fn main() { } let fs = Arc::new(RealFs::new(git_binary_path, app.background_executor())); - let user_settings_file_rx = watch_config_file( + let (user_settings_file_rx, user_settings_watcher) = watch_config_file( &app.background_executor(), fs.clone(), paths::settings_file().clone(), ); - let global_settings_file_rx = watch_config_file( + let (global_settings_file_rx, global_settings_watcher) = watch_config_file( &app.background_executor(), fs.clone(), paths::global_settings_file().clone(), ); - let user_keymap_file_rx = watch_config_file( + let (user_keymap_file_rx, user_keymap_watcher) = watch_config_file( &app.background_executor(), fs.clone(), paths::keymap_file().clone(), @@ -435,8 +435,14 @@ fn main() { } settings::init(cx); zlog_settings::init(cx); - handle_settings_file_changes(user_settings_file_rx, global_settings_file_rx, cx); - handle_keymap_file_changes(user_keymap_file_rx, cx); + handle_settings_file_changes( + user_settings_file_rx, + user_settings_watcher, + global_settings_file_rx, + global_settings_watcher, + cx, + ); + handle_keymap_file_changes(user_keymap_file_rx, user_keymap_watcher, cx); let user_agent = format!( "Zed/{} ({}; {})", diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 2becee37d105300dc58554ef594d4e2844332a74..1fbbc1a808365e1de6a06de475d68e6037f59dab 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -1542,7 +1542,9 @@ fn notify_settings_errors(result: settings::SettingsParseResult, is_user: bool, pub fn handle_settings_file_changes( mut user_settings_file_rx: mpsc::UnboundedReceiver, + user_settings_watcher: gpui::Task<()>, mut global_settings_file_rx: mpsc::UnboundedReceiver, + global_settings_watcher: gpui::Task<()>, cx: &mut App, ) { MigrationNotification::set_global(cx.new(|_| MigrationNotification), cx); @@ -1564,6 +1566,8 @@ pub fn handle_settings_file_changes( // Watch for changes in both files cx.spawn(async move |cx| { + let _user_settings_watcher = user_settings_watcher; + let _global_settings_watcher = global_settings_watcher; let mut settings_streams = futures::stream::select( global_settings_file_rx.map(Either::Left), user_settings_file_rx.map(Either::Right), @@ -1601,6 +1605,7 @@ pub fn handle_settings_file_changes( pub fn handle_keymap_file_changes( mut user_keymap_file_rx: mpsc::UnboundedReceiver, + user_keymap_watcher: gpui::Task<()>, cx: &mut App, ) { let (base_keymap_tx, mut base_keymap_rx) = mpsc::unbounded(); @@ -1659,6 +1664,7 @@ pub fn handle_keymap_file_changes( let notification_id = NotificationId::unique::(); cx.spawn(async move |cx| { + let _user_keymap_watcher = user_keymap_watcher; let mut user_keymap_content = String::new(); let mut migrating_in_memory = false; loop { @@ -4509,23 +4515,29 @@ mod tests { .unwrap(); executor.run_until_parked(); cx.update(|cx| { - let settings_rx = watch_config_file( + let (settings_rx, settings_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/settings.json"), ); - let keymap_rx = watch_config_file( + let (keymap_rx, keymap_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/keymap.json"), ); - let global_settings_rx = watch_config_file( + let (global_settings_rx, global_settings_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/global_settings.json"), ); - handle_settings_file_changes(settings_rx, global_settings_rx, cx); - handle_keymap_file_changes(keymap_rx, cx); + handle_settings_file_changes( + settings_rx, + settings_watcher, + global_settings_rx, + global_settings_watcher, + cx, + ); + handle_keymap_file_changes(keymap_rx, keymap_watcher, cx); }); workspace .update(cx, |workspace, _, cx| { @@ -4626,24 +4638,30 @@ mod tests { .unwrap(); cx.update(|cx| { - let settings_rx = watch_config_file( + let (settings_rx, settings_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/settings.json"), ); - let keymap_rx = watch_config_file( + let (keymap_rx, keymap_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/keymap.json"), ); - let global_settings_rx = watch_config_file( + let (global_settings_rx, global_settings_watcher) = watch_config_file( &executor, app_state.fs.clone(), PathBuf::from("/global_settings.json"), ); - handle_settings_file_changes(settings_rx, global_settings_rx, cx); - handle_keymap_file_changes(keymap_rx, cx); + handle_settings_file_changes( + settings_rx, + settings_watcher, + global_settings_rx, + global_settings_watcher, + cx, + ); + handle_keymap_file_changes(keymap_rx, keymap_watcher, cx); }); cx.background_executor.run_until_parked();