Merge pull request #532 from zed-industries/handle-language-server-failure

Max Brunsfeld created

Avoid infinite loop when a language server fails to start

Change summary

crates/language/src/buffer.rs   | 53 ++++++++++++++++------------------
crates/language/src/language.rs |  9 ++---
crates/language/src/tests.rs    | 21 +++++++++++++
crates/lsp/src/lsp.rs           | 12 ++++++-
crates/project/src/project.rs   | 23 ++++----------
5 files changed, 67 insertions(+), 51 deletions(-)

Detailed changes

crates/language/src/buffer.rs 🔗

@@ -132,7 +132,7 @@ struct LanguageServerState {
     latest_snapshot: watch::Sender<LanguageServerSnapshot>,
     pending_snapshots: BTreeMap<usize, LanguageServerSnapshot>,
     next_version: usize,
-    _maintain_server: Task<()>,
+    _maintain_server: Task<Option<()>>,
 }
 
 #[derive(Clone)]
@@ -589,33 +589,30 @@ impl Buffer {
                 next_version: 1,
                 server: server.clone(),
                 _maintain_server: cx.spawn_weak(|this, mut cx| async move {
-                    let mut capabilities = server.capabilities();
-                    loop {
-                        if let Some(capabilities) = capabilities.recv().await.flatten() {
-                            if let Some(this) = this.upgrade(&cx) {
-                                let triggers = capabilities
-                                    .completion_provider
-                                    .and_then(|c| c.trigger_characters)
-                                    .unwrap_or_default();
-                                this.update(&mut cx, |this, cx| {
-                                    let lamport_timestamp = this.text.lamport_clock.tick();
-                                    this.completion_triggers = triggers.clone();
-                                    this.send_operation(
-                                        Operation::UpdateCompletionTriggers {
-                                            triggers,
-                                            lamport_timestamp,
-                                        },
-                                        cx,
-                                    );
-                                    cx.notify();
-                                });
-                            } else {
-                                return;
-                            }
-
-                            break;
+                    let capabilities = server.capabilities().await.or_else(|| {
+                        log::info!("language server exited");
+                        if let Some(this) = this.upgrade(&cx) {
+                            this.update(&mut cx, |this, _| this.language_server = None);
                         }
-                    }
+                        None
+                    })?;
+
+                    let triggers = capabilities
+                        .completion_provider
+                        .and_then(|c| c.trigger_characters)
+                        .unwrap_or_default();
+                    this.upgrade(&cx)?.update(&mut cx, |this, cx| {
+                        let lamport_timestamp = this.text.lamport_clock.tick();
+                        this.completion_triggers = triggers.clone();
+                        this.send_operation(
+                            Operation::UpdateCompletionTriggers {
+                                triggers,
+                                lamport_timestamp,
+                            },
+                            cx,
+                        );
+                        cx.notify();
+                    });
 
                     let maintain_changes = cx.background().spawn(async move {
                         let initial_snapshot =
@@ -674,7 +671,7 @@ impl Buffer {
                         Ok::<_, anyhow::Error>(())
                     });
 
-                    maintain_changes.log_err().await;
+                    maintain_changes.log_err().await
                 }),
             })
         } else {

crates/language/src/language.rs 🔗

@@ -242,8 +242,6 @@ impl LanguageRegistry {
         #[cfg(any(test, feature = "test-support"))]
         if let Some(config) = &language.config.language_server {
             if let Some(fake_config) = &config.fake_config {
-                use postage::prelude::Stream;
-
                 let (server, mut fake_server) = lsp::LanguageServer::fake_with_capabilities(
                     fake_config.capabilities.clone(),
                     cx,
@@ -254,11 +252,12 @@ impl LanguageRegistry {
                 }
 
                 let servers_tx = fake_config.servers_tx.clone();
-                let mut initialized = server.capabilities();
+                let initialized = server.capabilities();
                 cx.background()
                     .spawn(async move {
-                        while initialized.recv().await.is_none() {}
-                        servers_tx.unbounded_send(fake_server).ok();
+                        if initialized.await.is_some() {
+                            servers_tx.unbounded_send(fake_server).ok();
+                        }
                     })
                     .detach();
 

crates/language/src/tests.rs 🔗

@@ -835,6 +835,27 @@ async fn test_diagnostics(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_language_server_has_exited(cx: &mut gpui::TestAppContext) {
+    let (language_server, fake) = cx.update(lsp::LanguageServer::fake);
+
+    // Simulate the language server failing to start up.
+    drop(fake);
+
+    let buffer = cx.add_model(|cx| {
+        Buffer::from_file(0, "", Box::new(FakeFile::new("/some/path")), cx)
+            .with_language(Arc::new(rust_lang()), cx)
+            .with_language_server(language_server, cx)
+    });
+
+    // Run the buffer's task that retrieves the server's capabilities.
+    cx.foreground().advance_clock(Duration::from_millis(1));
+
+    buffer.read_with(cx, |buffer, _| {
+        assert!(buffer.language_server().is_none());
+    });
+}
+
 #[gpui::test]
 async fn test_edits_from_lsp_with_past_version(cx: &mut gpui::TestAppContext) {
     let (language_server, mut fake) = cx.update(lsp::LanguageServer::fake);

crates/lsp/src/lsp.rs 🔗

@@ -378,8 +378,16 @@ impl LanguageServer {
         }
     }
 
-    pub fn capabilities(&self) -> watch::Receiver<Option<ServerCapabilities>> {
-        self.capabilities.clone()
+    pub fn capabilities(&self) -> impl 'static + Future<Output = Option<ServerCapabilities>> {
+        let mut rx = self.capabilities.clone();
+        async move {
+            loop {
+                let value = rx.recv().await?;
+                if value.is_some() {
+                    return value;
+                }
+            }
+        }
     }
 
     pub fn request<T: request::Request>(

crates/project/src/project.rs 🔗

@@ -22,7 +22,7 @@ use language::{
 };
 use lsp::{DiagnosticSeverity, DocumentHighlightKind, LanguageServer};
 use lsp_command::*;
-use postage::{prelude::Stream, watch};
+use postage::watch;
 use rand::prelude::*;
 use search::SearchQuery;
 use sha2::{Digest, Sha256};
@@ -1730,13 +1730,9 @@ impl Project {
                 range.end.to_point_utf16(buffer).to_lsp_position(),
             );
             cx.foreground().spawn(async move {
-                let mut capabilities = lang_server.capabilities();
-                while capabilities.borrow().is_none() {
-                    capabilities.recv().await;
-                }
-                if !capabilities
-                    .borrow()
-                    .as_ref()
+                if !lang_server
+                    .capabilities()
+                    .await
                     .map_or(false, |capabilities| {
                         capabilities.code_action_provider.is_some()
                     })
@@ -2266,14 +2262,9 @@ impl Project {
             if let Some((file, language_server)) = file.zip(buffer.language_server().cloned()) {
                 let lsp_params = request.to_lsp(&file.abs_path(cx), cx);
                 return cx.spawn(|this, cx| async move {
-                    let mut capabilities = language_server.capabilities();
-                    while capabilities.borrow().is_none() {
-                        capabilities.recv().await;
-                    }
-
-                    if !capabilities
-                        .borrow()
-                        .as_ref()
+                    if !language_server
+                        .capabilities()
+                        .await
                         .map_or(false, |capabilities| {
                             request.check_capabilities(&capabilities)
                         })