Fix file descriptors leak in evals (#18351)

Richard Feldman , Max , Piotr , and Piotr Osiewicz created

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 <max@zed.dev>
Co-authored-by: Piotr <piotr@zed.dev>
Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>

Change summary

Cargo.lock                                        |  1 
crates/gpui/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(-)

Detailed changes

Cargo.lock 🔗

@@ -10498,6 +10498,7 @@ dependencies = [
  "futures 0.3.30",
  "gpui",
  "parking_lot",
+ "paths",
  "serde",
  "serde_json",
  "snippet",

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");

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 {

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

crates/snippet_provider/src/lib.rs 🔗

@@ -130,8 +130,29 @@ async fn initial_scan(
 pub struct SnippetProvider {
     fs: Arc<dyn Fs>,
     snippets: HashMap<SnippetKind, BTreeMap<PathBuf, Vec<Arc<Snippet>>>>,
+    watch_tasks: Vec<Task<Result<()>>>,
 }
 
+// Watches global snippet directory, is created just once and reused across multiple projects
+struct GlobalSnippetWatcher(Model<SnippetProvider>);
+
+impl GlobalSnippetWatcher {
+    fn new(fs: Arc<dyn Fs>, 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<dyn Fs>,
@@ -139,29 +160,29 @@ impl SnippetProvider {
         cx: &mut AppContext,
     ) -> Model<Self> {
         cx.new_model(move |cx| {
+            if !cx.has_global::<GlobalSnippetWatcher>() {
+                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<Self>) -> Task<Result<()>> {
+    fn watch_directory(&mut self, path: &Path, cx: &mut ModelContext<Self>) {
         let path: Arc<Path> = 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::<GlobalSnippetWatcher>() {
+                user_snippets.extend(
+                    global_watcher
+                        .0
+                        .read(cx)
+                        .lookup_snippets::<false>(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<Arc<Snippet>> {
-        let mut requested_snippets = self.lookup_snippets(&language, cx);
+        let mut requested_snippets = self.lookup_snippets::<true>(&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::<true>(&None, cx));
         }
         requested_snippets
     }