From 9e4b3ce94c1db6ea3d2967b3f0e2a667a4939c3d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 28 Feb 2024 14:08:45 -0800 Subject: [PATCH] Avoid an unwrap when loading languages (#8562) We couldn't reproduce the panic, but I believe it was possible when uninstalling an extension while one if its grammars was still loading. Release Notes: - Fixed a crash that could happen when uninstalling a language extension while its grammar was loading. --------- Co-authored-by: Conrad --- crates/language/src/buffer_tests.rs | 12 ++++----- crates/language/src/language.rs | 6 ++--- crates/language/src/language_registry.rs | 34 +++++++++++++----------- crates/rpc/src/peer.rs | 3 +++ crates/zed/src/zed.rs | 14 +++++----- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 6bdd259c83..9750cec9bf 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -97,14 +97,14 @@ fn test_select_language() { // matching file extension assert_eq!( registry - .language_for_file("zed/lib.rs", None) + .language_for_file("zed/lib.rs".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), Some("Rust".into()) ); assert_eq!( registry - .language_for_file("zed/lib.mk", None) + .language_for_file("zed/lib.mk".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), Some("Make".into()) @@ -113,7 +113,7 @@ fn test_select_language() { // matching filename assert_eq!( registry - .language_for_file("zed/Makefile", None) + .language_for_file("zed/Makefile".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), Some("Make".into()) @@ -122,21 +122,21 @@ fn test_select_language() { // matching suffix that is not the full file extension or filename assert_eq!( registry - .language_for_file("zed/cars", None) + .language_for_file("zed/cars".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), None ); assert_eq!( registry - .language_for_file("zed/a.cars", None) + .language_for_file("zed/a.cars".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), None ); assert_eq!( registry - .language_for_file("zed/sumk", None) + .language_for_file("zed/sumk".as_ref(), None) .now_or_never() .and_then(|l| Some(l.ok()?.name())), None diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 0b884803a9..8d10bfc7f8 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -1522,16 +1522,16 @@ mod tests { }); languages - .language_for_file("the/script", None) + .language_for_file("the/script".as_ref(), None) .await .unwrap_err(); languages - .language_for_file("the/script", Some(&"nothing".into())) + .language_for_file("the/script".as_ref(), Some(&"nothing".into())) .await .unwrap_err(); assert_eq!( languages - .language_for_file("the/script", Some(&"#!/bin/env node".into())) + .language_for_file("the/script".as_ref(), Some(&"#!/bin/env node".into())) .await .unwrap() .name() diff --git a/crates/language/src/language_registry.rs b/crates/language/src/language_registry.rs index 82cf26d41d..95cdc3ff31 100644 --- a/crates/language/src/language_registry.rs +++ b/crates/language/src/language_registry.rs @@ -7,7 +7,7 @@ use collections::{hash_map, HashMap}; use futures::{ channel::{mpsc, oneshot}, future::Shared, - FutureExt as _, TryFutureExt as _, + Future, FutureExt as _, TryFutureExt as _, }; use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, Task}; use lsp::{LanguageServerBinary, LanguageServerId}; @@ -24,7 +24,7 @@ use sum_tree::Bias; use text::{Point, Rope}; use theme::Theme; use unicase::UniCase; -use util::{paths::PathExt, post_inc, ResultExt, TryFutureExt as _, UnwrapFuture}; +use util::{paths::PathExt, post_inc, ResultExt}; pub struct LanguageRegistry { state: RwLock, @@ -291,35 +291,36 @@ impl LanguageRegistry { pub fn language_for_name( self: &Arc, name: &str, - ) -> UnwrapFuture>>> { + ) -> impl Future>> { let name = UniCase::new(name); - self.get_or_load_language(|language_name, _| UniCase::new(language_name) == name) + let rx = self.get_or_load_language(|language_name, _| UniCase::new(language_name) == name); + async move { rx.await? } } pub fn language_for_name_or_extension( self: &Arc, string: &str, - ) -> UnwrapFuture>>> { + ) -> impl Future>> { let string = UniCase::new(string); - self.get_or_load_language(|name, config| { + let rx = self.get_or_load_language(|name, config| { UniCase::new(name) == string || config .path_suffixes .iter() .any(|suffix| UniCase::new(suffix) == string) - }) + }); + async move { rx.await? } } pub fn language_for_file( self: &Arc, - path: impl AsRef, + path: &Path, content: Option<&Rope>, - ) -> UnwrapFuture>>> { - let path = path.as_ref(); + ) -> impl Future>> { let filename = path.file_name().and_then(|name| name.to_str()); let extension = path.extension_or_hidden_file_name(); let path_suffixes = [extension, filename]; - self.get_or_load_language(|_, config| { + let rx = self.get_or_load_language(move |_, config| { let path_matches = config .path_suffixes .iter() @@ -334,13 +335,14 @@ impl LanguageRegistry { }, ); path_matches || content_matches - }) + }); + async move { rx.await? } } fn get_or_load_language( self: &Arc, callback: impl Fn(&str, &LanguageMatcher) -> bool, - ) -> UnwrapFuture>>> { + ) -> oneshot::Receiver>> { let (tx, rx) = oneshot::channel(); let mut state = self.state.write(); @@ -421,13 +423,13 @@ impl LanguageRegistry { let _ = tx.send(Err(anyhow!("executor does not exist"))); } - rx.unwrap() + rx } fn get_or_load_grammar( self: &Arc, name: Arc, - ) -> UnwrapFuture>> { + ) -> impl Future> { let (tx, rx) = oneshot::channel(); let mut state = self.state.write(); @@ -483,7 +485,7 @@ impl LanguageRegistry { tx.send(Err(anyhow!("no such grammar {}", name))).ok(); } - rx.unwrap() + async move { rx.await? } } pub fn to_vec(&self) -> Vec> { diff --git a/crates/rpc/src/peer.rs b/crates/rpc/src/peer.rs index f3f74899b9..4222404a47 100644 --- a/crates/rpc/src/peer.rs +++ b/crates/rpc/src/peer.rs @@ -315,10 +315,13 @@ impl Peer { "incoming response: requester resumed" ); } else { + let message_type = proto::build_typed_envelope(connection_id, incoming) + .map(|p| p.payload_type_name()); tracing::warn!( %connection_id, message_id, responding_to, + message_type, "incoming response: unknown request" ); } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 2d8c8649ac..604bb99968 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2751,18 +2751,20 @@ mod tests { } #[gpui::test] - fn test_bundled_languages(cx: &mut AppContext) { - let settings = SettingsStore::test(cx); + async fn test_bundled_languages(cx: &mut TestAppContext) { + let settings = cx.update(|cx| SettingsStore::test(cx)); cx.set_global(settings); let mut languages = LanguageRegistry::test(); - languages.set_executor(cx.background_executor().clone()); + languages.set_executor(cx.executor().clone()); let languages = Arc::new(languages); let node_runtime = node_runtime::FakeNodeRuntime::new(); - languages::init(languages.clone(), node_runtime, cx); + cx.update(|cx| { + languages::init(languages.clone(), node_runtime, cx); + }); for name in languages.language_names() { - languages.language_for_name(&name); + languages.language_for_name(&name).await.unwrap(); } - cx.background_executor().run_until_parked(); + cx.run_until_parked(); } fn init_test(cx: &mut TestAppContext) -> Arc {