From 1eddd2f38d844f50af4ff0e76ddab63687519d7f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 25 Sep 2024 15:21:00 -0400 Subject: [PATCH] Fix file descriptors leak in evals (#18351) Fixes an issue where evals were hitting "too many open files" errors because we were adding (and detaching) new directory watches for each project. Now we add those watches globally/at the worktree level, and we store the tasks so they stop watching on drop. Release Notes: - N/A --------- Co-authored-by: Max Co-authored-by: Piotr Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> --- Cargo.lock | 1 + .../src/platform/mac/attributed_string.rs | 4 +- crates/project/src/project.rs | 10 +--- crates/snippet_provider/Cargo.toml | 1 + crates/snippet_provider/src/lib.rs | 55 +++++++++++++++---- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41b2d6d452..26b979ccf7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10498,6 +10498,7 @@ dependencies = [ "futures 0.3.30", "gpui", "parking_lot", + "paths", "serde", "serde_json", "snippet", diff --git a/crates/gpui/src/platform/mac/attributed_string.rs b/crates/gpui/src/platform/mac/attributed_string.rs index 663ce67d4c..3f1185bc14 100644 --- a/crates/gpui/src/platform/mac/attributed_string.rs +++ b/crates/gpui/src/platform/mac/attributed_string.rs @@ -70,9 +70,7 @@ mod tests { unsafe { let image: id = msg_send![class!(NSImage), alloc]; - image.initWithContentsOfFile_( - NSString::alloc(nil).init_str("/Users/rtfeldman/Downloads/test.jpeg"), - ); + image.initWithContentsOfFile_(NSString::alloc(nil).init_str("test.jpeg")); let _size = image.size(); let string = NSString::alloc(nil).init_str("Test String"); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8d95c8f2f1..10fd88f286 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -587,10 +587,7 @@ impl Project { cx.spawn(move |this, cx| Self::send_buffer_ordered_messages(this, rx, cx)) .detach(); let tasks = Inventory::new(cx); - let global_snippets_dir = paths::config_dir().join("snippets"); - let snippets = - SnippetProvider::new(fs.clone(), BTreeSet::from_iter([global_snippets_dir]), cx); - + let snippets = SnippetProvider::new(fs.clone(), BTreeSet::from_iter([]), cx); let worktree_store = cx.new_model(|_| WorktreeStore::local(false, fs.clone())); cx.subscribe(&worktree_store, Self::on_worktree_store_event) .detach(); @@ -875,9 +872,8 @@ impl Project { let this = cx.new_model(|cx| { let replica_id = response.payload.replica_id as ReplicaId; let tasks = Inventory::new(cx); - let global_snippets_dir = paths::config_dir().join("snippets"); - let snippets = - SnippetProvider::new(fs.clone(), BTreeSet::from_iter([global_snippets_dir]), cx); + + let snippets = SnippetProvider::new(fs.clone(), BTreeSet::from_iter([]), cx); let mut worktrees = Vec::new(); for worktree in response.payload.worktrees { diff --git a/crates/snippet_provider/Cargo.toml b/crates/snippet_provider/Cargo.toml index 75b7210a7a..95ab19ebb6 100644 --- a/crates/snippet_provider/Cargo.toml +++ b/crates/snippet_provider/Cargo.toml @@ -15,6 +15,7 @@ fs.workspace = true futures.workspace = true gpui.workspace = true parking_lot.workspace = true +paths.workspace = true serde.workspace = true serde_json.workspace = true snippet.workspace = true diff --git a/crates/snippet_provider/src/lib.rs b/crates/snippet_provider/src/lib.rs index 17d615866a..a18f9ff1b6 100644 --- a/crates/snippet_provider/src/lib.rs +++ b/crates/snippet_provider/src/lib.rs @@ -130,8 +130,29 @@ async fn initial_scan( pub struct SnippetProvider { fs: Arc, snippets: HashMap>>>, + watch_tasks: Vec>>, } +// Watches global snippet directory, is created just once and reused across multiple projects +struct GlobalSnippetWatcher(Model); + +impl GlobalSnippetWatcher { + fn new(fs: Arc, cx: &mut AppContext) -> Self { + let global_snippets_dir = paths::config_dir().join("snippets"); + let provider = cx.new_model(|_cx| SnippetProvider { + fs, + snippets: Default::default(), + watch_tasks: vec![], + }); + provider.update(cx, |this, cx| { + this.watch_directory(&global_snippets_dir, cx) + }); + Self(provider) + } +} + +impl gpui::Global for GlobalSnippetWatcher {} + impl SnippetProvider { pub fn new( fs: Arc, @@ -139,29 +160,29 @@ impl SnippetProvider { cx: &mut AppContext, ) -> Model { cx.new_model(move |cx| { + if !cx.has_global::() { + let global_watcher = GlobalSnippetWatcher::new(fs.clone(), cx); + cx.set_global(global_watcher); + } let mut this = Self { fs, + watch_tasks: Vec::new(), snippets: Default::default(), }; - let mut task_handles = vec![]; for dir in dirs_to_watch { - task_handles.push(this.watch_directory(&dir, cx)); + this.watch_directory(&dir, cx); } - cx.spawn(|_, _| async move { - futures::future::join_all(task_handles).await; - }) - .detach(); this }) } /// Add directory to be watched for content changes - fn watch_directory(&mut self, path: &Path, cx: &mut ModelContext) -> Task> { + fn watch_directory(&mut self, path: &Path, cx: &mut ModelContext) { let path: Arc = Arc::from(path); - cx.spawn(|this, mut cx| async move { + self.watch_tasks.push(cx.spawn(|this, mut cx| async move { let fs = this.update(&mut cx, |this, _| this.fs.clone())?; let watched_path = path.clone(); let watcher = fs.watch(&watched_path, Duration::from_secs(1)); @@ -177,10 +198,10 @@ impl SnippetProvider { .await?; } Ok(()) - }) + })); } - fn lookup_snippets<'a>( + fn lookup_snippets<'a, const LOOKUP_GLOBALS: bool>( &'a self, language: &'a SnippetKind, cx: &AppContext, @@ -193,6 +214,16 @@ impl SnippetProvider { .into_iter() .flat_map(|(_, snippets)| snippets.into_iter()) .collect(); + if LOOKUP_GLOBALS { + if let Some(global_watcher) = cx.try_global::() { + user_snippets.extend( + global_watcher + .0 + .read(cx) + .lookup_snippets::(language, cx), + ); + } + } let Some(registry) = SnippetRegistry::try_global(cx) else { return user_snippets; @@ -205,11 +236,11 @@ impl SnippetProvider { } pub fn snippets_for(&self, language: SnippetKind, cx: &AppContext) -> Vec> { - let mut requested_snippets = self.lookup_snippets(&language, cx); + let mut requested_snippets = self.lookup_snippets::(&language, cx); if language.is_some() { // Look up global snippets as well. - requested_snippets.extend(self.lookup_snippets(&None, cx)); + requested_snippets.extend(self.lookup_snippets::(&None, cx)); } requested_snippets }