From e2f6422b3e1b4c1901523d9000c136e9d624fa7e Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Fri, 21 Nov 2025 14:19:55 +0530 Subject: [PATCH] language: Move language server update to background when it takes too long (#43164) Closes https://github.com/zed-industries/zed/issues/42360 If updating a language server takes longer than 10 seconds, we now fall back to launching the currently installed version (if exists) and continue downloading the update in the background. Release Notes: - Improved language server updates for slow connection, now Zed launches existing server if the update is taking too long. --------- Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> --- crates/language/src/language.rs | 151 ++++++++++-------- .../src/extension_lsp_adapter.rs | 79 ++++----- crates/languages/src/css.rs | 17 +- crates/languages/src/json.rs | 14 +- crates/languages/src/tailwind.rs | 17 +- crates/languages/src/yaml.rs | 17 +- crates/project/src/lsp_store.rs | 27 +++- 7 files changed, 163 insertions(+), 159 deletions(-) diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 7ce3986736cc0a1e8b8d21124ebe8c29ddc9214c..03e563e145c3bd1cde63e62fa8a09a4fb0228f0f 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -28,6 +28,8 @@ use anyhow::{Context as _, Result}; use async_trait::async_trait; use collections::{HashMap, HashSet, IndexSet}; use futures::Future; +use futures::future::LocalBoxFuture; +use futures::lock::OwnedMutexGuard; use gpui::{App, AsyncApp, Entity, SharedString}; pub use highlight_map::HighlightMap; use http_client::HttpClient; @@ -51,7 +53,6 @@ use std::{ mem, ops::{DerefMut, Range}, path::{Path, PathBuf}, - pin::Pin, str, sync::{ Arc, LazyLock, @@ -152,7 +153,14 @@ pub struct Location { } type ServerBinaryCache = futures::lock::Mutex>; - +type DownloadableLanguageServerBinary = LocalBoxFuture<'static, Result>; +pub type LanguageServerBinaryLocations = LocalBoxFuture< + 'static, + ( + Result, + Option, + ), +>; /// Represents a Language Server, with certain cached sync properties. /// Uses [`LspAdapter`] under the hood, but calls all 'static' methods /// once at startup, and caches the results. @@ -162,7 +170,7 @@ pub struct CachedLspAdapter { pub disk_based_diagnostics_progress_token: Option, language_ids: HashMap, pub adapter: Arc, - cached_binary: ServerBinaryCache, + cached_binary: Arc, } impl Debug for CachedLspAdapter { @@ -209,18 +217,15 @@ impl CachedLspAdapter { toolchains: Option, binary_options: LanguageServerBinaryOptions, cx: &mut AsyncApp, - ) -> Result { - let mut cached_binary = self.cached_binary.lock().await; - self.adapter - .clone() - .get_language_server_command( - delegate, - toolchains, - binary_options, - &mut cached_binary, - cx, - ) - .await + ) -> LanguageServerBinaryLocations { + let cached_binary = self.cached_binary.clone().lock_owned().await; + self.adapter.clone().get_language_server_command( + delegate, + toolchains, + binary_options, + cached_binary, + cx.clone(), + ) } pub fn code_action_kinds(&self) -> Option> { @@ -513,14 +518,14 @@ pub trait DynLspInstaller { pre_release: bool, cx: &mut AsyncApp, ) -> Result; - fn get_language_server_command<'a>( + fn get_language_server_command( self: Arc, delegate: Arc, toolchains: Option, binary_options: LanguageServerBinaryOptions, - cached_binary: &'a mut Option<(bool, LanguageServerBinary)>, - cx: &'a mut AsyncApp, - ) -> Pin>>>; + cached_binary: OwnedMutexGuard>, + cx: AsyncApp, + ) -> LanguageServerBinaryLocations; } #[async_trait(?Send)] @@ -562,15 +567,16 @@ where binary } } - fn get_language_server_command<'a>( + fn get_language_server_command( self: Arc, delegate: Arc, toolchain: Option, binary_options: LanguageServerBinaryOptions, - cached_binary: &'a mut Option<(bool, LanguageServerBinary)>, - cx: &'a mut AsyncApp, - ) -> Pin>>> { + mut cached_binary: OwnedMutexGuard>, + mut cx: AsyncApp, + ) -> LanguageServerBinaryLocations { async move { + let cached_binary_deref = cached_binary.deref_mut(); // First we check whether the adapter can give us a user-installed binary. // If so, we do *not* want to cache that, because each worktree might give us a different // binary: @@ -584,7 +590,7 @@ where // for each worktree we might have open. if binary_options.allow_path_lookup && let Some(binary) = self - .check_if_user_installed(delegate.as_ref(), toolchain, cx) + .check_if_user_installed(delegate.as_ref(), toolchain, &mut cx) .await { log::info!( @@ -593,62 +599,77 @@ where binary.path, binary.arguments ); - return Ok(binary); + return (Ok(binary), None); } - anyhow::ensure!( - binary_options.allow_binary_download, - "downloading language servers disabled" - ); + if !binary_options.allow_binary_download { + return ( + Err(anyhow::anyhow!("downloading language servers disabled")), + None, + ); + } - if let Some((pre_release, cached_binary)) = cached_binary + if let Some((pre_release, cached_binary)) = cached_binary_deref && *pre_release == binary_options.pre_release { - return Ok(cached_binary.clone()); + return (Ok(cached_binary.clone()), None); } let Some(container_dir) = delegate.language_server_download_dir(&self.name()).await else { - anyhow::bail!("no language server download dir defined") + return ( + Err(anyhow::anyhow!("no language server download dir defined")), + None, + ); }; - let mut binary = self - .try_fetch_server_binary( - &delegate, - container_dir.to_path_buf(), - binary_options.pre_release, - cx, - ) - .await; + let last_downloaded_binary = self + .cached_server_binary(container_dir.to_path_buf(), delegate.as_ref()) + .await + .context( + "did not find existing language server binary, falling back to downloading", + ); + let download_binary = async move { + let mut binary = self + .try_fetch_server_binary( + &delegate, + container_dir.to_path_buf(), + binary_options.pre_release, + &mut cx, + ) + .await; + + if let Err(error) = binary.as_ref() { + if let Some(prev_downloaded_binary) = self + .cached_server_binary(container_dir.to_path_buf(), delegate.as_ref()) + .await + { + log::info!( + "failed to fetch newest version of language server {:?}. \ + error: {:?}, falling back to using {:?}", + self.name(), + error, + prev_downloaded_binary.path + ); + binary = Ok(prev_downloaded_binary); + } else { + delegate.update_status( + self.name(), + BinaryStatus::Failed { + error: format!("{error:?}"), + }, + ); + } + } - if let Err(error) = binary.as_ref() { - if let Some(prev_downloaded_binary) = self - .cached_server_binary(container_dir.to_path_buf(), delegate.as_ref()) - .await - { - log::info!( - "failed to fetch newest version of language server {:?}. \ - error: {:?}, falling back to using {:?}", - self.name(), - error, - prev_downloaded_binary.path - ); - binary = Ok(prev_downloaded_binary); - } else { - delegate.update_status( - self.name(), - BinaryStatus::Failed { - error: format!("{error:?}"), - }, - ); + if let Ok(binary) = &binary { + *cached_binary = Some((binary_options.pre_release, binary.clone())); } - } - if let Ok(binary) = &binary { - *cached_binary = Some((binary_options.pre_release, binary.clone())); + binary } - - binary + .boxed_local(); + (last_downloaded_binary, Some(download_binary)) } .boxed_local() } diff --git a/crates/language_extension/src/extension_lsp_adapter.rs b/crates/language_extension/src/extension_lsp_adapter.rs index 01b726748649e29b4fe69ce26df5564819894985..566e6f3b3647bd7fbe4317590ddbe2fbe79890c2 100644 --- a/crates/language_extension/src/extension_lsp_adapter.rs +++ b/crates/language_extension/src/extension_lsp_adapter.rs @@ -1,17 +1,16 @@ use std::ops::Range; use std::path::PathBuf; -use std::pin::Pin; use std::sync::Arc; use anyhow::{Context as _, Result}; use async_trait::async_trait; use collections::{HashMap, HashSet}; use extension::{Extension, ExtensionLanguageServerProxy, WorktreeDelegate}; -use futures::{Future, FutureExt, future::join_all}; +use futures::{FutureExt, future::join_all, lock::OwnedMutexGuard}; use gpui::{App, AppContext, AsyncApp, Task}; use language::{ - BinaryStatus, CodeLabel, DynLspInstaller, HighlightId, Language, LanguageName, LspAdapter, - LspAdapterDelegate, Toolchain, + BinaryStatus, CodeLabel, DynLspInstaller, HighlightId, Language, LanguageName, + LanguageServerBinaryLocations, LspAdapter, LspAdapterDelegate, Toolchain, }; use lsp::{ CodeActionKind, LanguageServerBinary, LanguageServerBinaryOptions, LanguageServerName, @@ -155,47 +154,51 @@ impl ExtensionLspAdapter { #[async_trait(?Send)] impl DynLspInstaller for ExtensionLspAdapter { - fn get_language_server_command<'a>( + fn get_language_server_command( self: Arc, delegate: Arc, _: Option, _: LanguageServerBinaryOptions, - _: &'a mut Option<(bool, LanguageServerBinary)>, - _: &'a mut AsyncApp, - ) -> Pin>>> { + _: OwnedMutexGuard>, + _: AsyncApp, + ) -> LanguageServerBinaryLocations { async move { - let delegate = Arc::new(WorktreeDelegateAdapter(delegate.clone())) as _; - let command = self - .extension - .language_server_command( - self.language_server_id.clone(), - self.language_name.clone(), - delegate, - ) - .await?; - - let path = self.extension.path_from_extension(command.command.as_ref()); - - // TODO: This should now be done via the `zed::make_file_executable` function in - // Zed extension API, but we're leaving these existing usages in place temporarily - // to avoid any compatibility issues between Zed and the extension versions. - // - // We can remove once the following extension versions no longer see any use: - // - toml@0.0.2 - // - zig@0.0.1 - if ["toml", "zig"].contains(&self.extension.manifest().id.as_ref()) - && path.starts_with(&self.extension.work_dir()) - { - make_file_executable(&path) - .await - .context("failed to set file permissions")?; - } + let ret = maybe!(async move { + let delegate = Arc::new(WorktreeDelegateAdapter(delegate.clone())) as _; + let command = self + .extension + .language_server_command( + self.language_server_id.clone(), + self.language_name.clone(), + delegate, + ) + .await?; + + let path = self.extension.path_from_extension(command.command.as_ref()); + + // TODO: This should now be done via the `zed::make_file_executable` function in + // Zed extension API, but we're leaving these existing usages in place temporarily + // to avoid any compatibility issues between Zed and the extension versions. + // + // We can remove once the following extension versions no longer see any use: + // - toml@0.0.2 + // - zig@0.0.1 + if ["toml", "zig"].contains(&self.extension.manifest().id.as_ref()) + && path.starts_with(&self.extension.work_dir()) + { + make_file_executable(&path) + .await + .context("failed to set file permissions")?; + } - Ok(LanguageServerBinary { - path, - arguments: command.args.into_iter().map(|arg| arg.into()).collect(), - env: Some(command.env.into_iter().collect()), + Ok(LanguageServerBinary { + path, + arguments: command.args.into_iter().map(|arg| arg.into()).collect(), + env: Some(command.env.into_iter().collect()), + }) }) + .await; + (ret, None) } .boxed_local() } diff --git a/crates/languages/src/css.rs b/crates/languages/src/css.rs index 035a2c693dbbdceed38adc8ccc0510274205670f..8531cd447827e77593edb818e24371c031100b64 100644 --- a/crates/languages/src/css.rs +++ b/crates/languages/src/css.rs @@ -1,13 +1,11 @@ -use anyhow::{Context as _, Result}; +use anyhow::Result; use async_trait::async_trait; -use futures::StreamExt; use gpui::AsyncApp; use language::{LspAdapter, LspAdapterDelegate, LspInstaller, Toolchain}; use lsp::{LanguageServerBinary, LanguageServerName}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; use serde_json::json; -use smol::fs; use std::{ ffi::OsString, path::{Path, PathBuf}, @@ -176,19 +174,10 @@ async fn get_cached_server_binary( node: &NodeRuntime, ) -> Option { maybe!(async { - let mut last_version_dir = None; - let mut entries = fs::read_dir(&container_dir).await?; - while let Some(entry) = entries.next().await { - let entry = entry?; - if entry.file_type().await?.is_dir() { - last_version_dir = Some(entry.path()); - } - } - let last_version_dir = last_version_dir.context("no cached binary")?; - let server_path = last_version_dir.join(SERVER_PATH); + let server_path = container_dir.join(SERVER_PATH); anyhow::ensure!( server_path.exists(), - "missing executable in directory {last_version_dir:?}" + "missing executable in directory {server_path:?}" ); Ok(LanguageServerBinary { path: node.binary_path().await?, diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 45fa2dd75cce051439980b40996dda9865246a99..d75d994fee76bfa696b9501f06c5cca23eb2484c 100644 --- a/crates/languages/src/json.rs +++ b/crates/languages/src/json.rs @@ -301,20 +301,10 @@ async fn get_cached_server_binary( node: &NodeRuntime, ) -> Option { maybe!(async { - let mut last_version_dir = None; - let mut entries = fs::read_dir(&container_dir).await?; - while let Some(entry) = entries.next().await { - let entry = entry?; - if entry.file_type().await?.is_dir() { - last_version_dir = Some(entry.path()); - } - } - - let last_version_dir = last_version_dir.context("no cached binary")?; - let server_path = last_version_dir.join(SERVER_PATH); + let server_path = container_dir.join(SERVER_PATH); anyhow::ensure!( server_path.exists(), - "missing executable in directory {last_version_dir:?}" + "missing executable in directory {server_path:?}" ); Ok(LanguageServerBinary { path: node.binary_path().await?, diff --git a/crates/languages/src/tailwind.rs b/crates/languages/src/tailwind.rs index e1b50a5ccaabb7770d13abc79fbac1da5fa4cbbe..c7baa9734b56bd139ccc69b197a9a772fd7aeec1 100644 --- a/crates/languages/src/tailwind.rs +++ b/crates/languages/src/tailwind.rs @@ -1,14 +1,12 @@ -use anyhow::{Context as _, Result}; +use anyhow::Result; use async_trait::async_trait; use collections::HashMap; -use futures::StreamExt; use gpui::AsyncApp; use language::{LanguageName, LspAdapter, LspAdapterDelegate, LspInstaller, Toolchain}; use lsp::{LanguageServerBinary, LanguageServerName}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; use serde_json::{Value, json}; -use smol::fs; use std::{ ffi::OsString, path::{Path, PathBuf}, @@ -198,19 +196,10 @@ async fn get_cached_server_binary( node: &NodeRuntime, ) -> Option { maybe!(async { - let mut last_version_dir = None; - let mut entries = fs::read_dir(&container_dir).await?; - while let Some(entry) = entries.next().await { - let entry = entry?; - if entry.file_type().await?.is_dir() { - last_version_dir = Some(entry.path()); - } - } - let last_version_dir = last_version_dir.context("no cached binary")?; - let server_path = last_version_dir.join(SERVER_PATH); + let server_path = container_dir.join(SERVER_PATH); anyhow::ensure!( server_path.exists(), - "missing executable in directory {last_version_dir:?}" + "missing executable in directory {server_path:?}" ); Ok(LanguageServerBinary { path: node.binary_path().await?, diff --git a/crates/languages/src/yaml.rs b/crates/languages/src/yaml.rs index 45faa142369e6c08817deebfbf8774f228bf70d5..c79f029491b09e32ffbbfa5cfaf00c74ed186978 100644 --- a/crates/languages/src/yaml.rs +++ b/crates/languages/src/yaml.rs @@ -1,6 +1,5 @@ -use anyhow::{Context as _, Result}; +use anyhow::Result; use async_trait::async_trait; -use futures::StreamExt; use gpui::AsyncApp; use language::{ LspAdapter, LspAdapterDelegate, LspInstaller, Toolchain, language_settings::AllLanguageSettings, @@ -10,7 +9,6 @@ use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; use serde_json::Value; use settings::{Settings, SettingsLocation}; -use smol::fs; use std::{ ffi::OsString, path::{Path, PathBuf}, @@ -171,19 +169,10 @@ async fn get_cached_server_binary( node: &NodeRuntime, ) -> Option { maybe!(async { - let mut last_version_dir = None; - let mut entries = fs::read_dir(&container_dir).await?; - while let Some(entry) = entries.next().await { - let entry = entry?; - if entry.file_type().await?.is_dir() { - last_version_dir = Some(entry.path()); - } - } - let last_version_dir = last_version_dir.context("no cached binary")?; - let server_path = last_version_dir.join(SERVER_PATH); + let server_path = container_dir.join(SERVER_PATH); anyhow::ensure!( server_path.exists(), - "missing executable in directory {last_version_dir:?}" + "missing executable in directory {server_path:?}" ); Ok(LanguageServerBinary { path: node.binary_path().await?, diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 17f558d72d4854bb99676472100c442ad164f0a5..e233ff1c5c121e301a85d829093cbdd37020fe07 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -139,6 +139,7 @@ pub use worktree::{ const SERVER_LAUNCHING_BEFORE_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); pub const SERVER_PROGRESS_THROTTLE_TIMEOUT: Duration = Duration::from_millis(100); const WORKSPACE_DIAGNOSTICS_TOKEN_START: &str = "id:"; +const SERVER_DOWNLOAD_TIMEOUT: Duration = Duration::from_secs(10); #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)] pub enum ProgressToken { @@ -599,14 +600,36 @@ impl LocalLspStore { }; cx.spawn(async move |cx| { - let binary_result = adapter + let (existing_binary, maybe_download_binary) = adapter .clone() .get_language_server_command(delegate.clone(), toolchain, lsp_binary_options, cx) + .await .await; delegate.update_status(adapter.name.clone(), BinaryStatus::None); - let mut binary = binary_result?; + let mut binary = match (existing_binary, maybe_download_binary) { + (binary, None) => binary?, + (Err(_), Some(downloader)) => downloader.await?, + (Ok(existing_binary), Some(downloader)) => { + let mut download_timeout = cx + .background_executor() + .timer(SERVER_DOWNLOAD_TIMEOUT) + .fuse(); + let mut downloader = downloader.fuse(); + futures::select! { + _ = download_timeout => { + // Return existing binary and kick the existing work to the background. + cx.spawn(async move |_| downloader.await).detach(); + Ok(existing_binary) + }, + downloaded_or_existing_binary = downloader => { + // If download fails, this results in the existing binary. + downloaded_or_existing_binary + } + }? + } + }; let mut shell_env = delegate.shell_env().await; shell_env.extend(binary.env.unwrap_or_default());