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 <conrad@zed.dev>
This commit is contained in:
Max Brunsfeld 2024-02-28 14:08:45 -08:00 committed by GitHub
parent 4a4ca2c3b8
commit 9e4b3ce94c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 38 additions and 31 deletions

View file

@ -97,14 +97,14 @@ fn test_select_language() {
// matching file extension // matching file extension
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/lib.rs", None) .language_for_file("zed/lib.rs".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
Some("Rust".into()) Some("Rust".into())
); );
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/lib.mk", None) .language_for_file("zed/lib.mk".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
Some("Make".into()) Some("Make".into())
@ -113,7 +113,7 @@ fn test_select_language() {
// matching filename // matching filename
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/Makefile", None) .language_for_file("zed/Makefile".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
Some("Make".into()) Some("Make".into())
@ -122,21 +122,21 @@ fn test_select_language() {
// matching suffix that is not the full file extension or filename // matching suffix that is not the full file extension or filename
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/cars", None) .language_for_file("zed/cars".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
None None
); );
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/a.cars", None) .language_for_file("zed/a.cars".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
None None
); );
assert_eq!( assert_eq!(
registry registry
.language_for_file("zed/sumk", None) .language_for_file("zed/sumk".as_ref(), None)
.now_or_never() .now_or_never()
.and_then(|l| Some(l.ok()?.name())), .and_then(|l| Some(l.ok()?.name())),
None None

View file

@ -1522,16 +1522,16 @@ mod tests {
}); });
languages languages
.language_for_file("the/script", None) .language_for_file("the/script".as_ref(), None)
.await .await
.unwrap_err(); .unwrap_err();
languages languages
.language_for_file("the/script", Some(&"nothing".into())) .language_for_file("the/script".as_ref(), Some(&"nothing".into()))
.await .await
.unwrap_err(); .unwrap_err();
assert_eq!( assert_eq!(
languages languages
.language_for_file("the/script", Some(&"#!/bin/env node".into())) .language_for_file("the/script".as_ref(), Some(&"#!/bin/env node".into()))
.await .await
.unwrap() .unwrap()
.name() .name()

View file

@ -7,7 +7,7 @@ use collections::{hash_map, HashMap};
use futures::{ use futures::{
channel::{mpsc, oneshot}, channel::{mpsc, oneshot},
future::Shared, future::Shared,
FutureExt as _, TryFutureExt as _, Future, FutureExt as _, TryFutureExt as _,
}; };
use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, Task}; use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, Task};
use lsp::{LanguageServerBinary, LanguageServerId}; use lsp::{LanguageServerBinary, LanguageServerId};
@ -24,7 +24,7 @@ use sum_tree::Bias;
use text::{Point, Rope}; use text::{Point, Rope};
use theme::Theme; use theme::Theme;
use unicase::UniCase; use unicase::UniCase;
use util::{paths::PathExt, post_inc, ResultExt, TryFutureExt as _, UnwrapFuture}; use util::{paths::PathExt, post_inc, ResultExt};
pub struct LanguageRegistry { pub struct LanguageRegistry {
state: RwLock<LanguageRegistryState>, state: RwLock<LanguageRegistryState>,
@ -291,35 +291,36 @@ impl LanguageRegistry {
pub fn language_for_name( pub fn language_for_name(
self: &Arc<Self>, self: &Arc<Self>,
name: &str, name: &str,
) -> UnwrapFuture<oneshot::Receiver<Result<Arc<Language>>>> { ) -> impl Future<Output = Result<Arc<Language>>> {
let name = UniCase::new(name); 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( pub fn language_for_name_or_extension(
self: &Arc<Self>, self: &Arc<Self>,
string: &str, string: &str,
) -> UnwrapFuture<oneshot::Receiver<Result<Arc<Language>>>> { ) -> impl Future<Output = Result<Arc<Language>>> {
let string = UniCase::new(string); 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 UniCase::new(name) == string
|| config || config
.path_suffixes .path_suffixes
.iter() .iter()
.any(|suffix| UniCase::new(suffix) == string) .any(|suffix| UniCase::new(suffix) == string)
}) });
async move { rx.await? }
} }
pub fn language_for_file( pub fn language_for_file(
self: &Arc<Self>, self: &Arc<Self>,
path: impl AsRef<Path>, path: &Path,
content: Option<&Rope>, content: Option<&Rope>,
) -> UnwrapFuture<oneshot::Receiver<Result<Arc<Language>>>> { ) -> impl Future<Output = Result<Arc<Language>>> {
let path = path.as_ref();
let filename = path.file_name().and_then(|name| name.to_str()); let filename = path.file_name().and_then(|name| name.to_str());
let extension = path.extension_or_hidden_file_name(); let extension = path.extension_or_hidden_file_name();
let path_suffixes = [extension, filename]; let path_suffixes = [extension, filename];
self.get_or_load_language(|_, config| { let rx = self.get_or_load_language(move |_, config| {
let path_matches = config let path_matches = config
.path_suffixes .path_suffixes
.iter() .iter()
@ -334,13 +335,14 @@ impl LanguageRegistry {
}, },
); );
path_matches || content_matches path_matches || content_matches
}) });
async move { rx.await? }
} }
fn get_or_load_language( fn get_or_load_language(
self: &Arc<Self>, self: &Arc<Self>,
callback: impl Fn(&str, &LanguageMatcher) -> bool, callback: impl Fn(&str, &LanguageMatcher) -> bool,
) -> UnwrapFuture<oneshot::Receiver<Result<Arc<Language>>>> { ) -> oneshot::Receiver<Result<Arc<Language>>> {
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let mut state = self.state.write(); let mut state = self.state.write();
@ -421,13 +423,13 @@ impl LanguageRegistry {
let _ = tx.send(Err(anyhow!("executor does not exist"))); let _ = tx.send(Err(anyhow!("executor does not exist")));
} }
rx.unwrap() rx
} }
fn get_or_load_grammar( fn get_or_load_grammar(
self: &Arc<Self>, self: &Arc<Self>,
name: Arc<str>, name: Arc<str>,
) -> UnwrapFuture<oneshot::Receiver<Result<tree_sitter::Language>>> { ) -> impl Future<Output = Result<tree_sitter::Language>> {
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let mut state = self.state.write(); let mut state = self.state.write();
@ -483,7 +485,7 @@ impl LanguageRegistry {
tx.send(Err(anyhow!("no such grammar {}", name))).ok(); tx.send(Err(anyhow!("no such grammar {}", name))).ok();
} }
rx.unwrap() async move { rx.await? }
} }
pub fn to_vec(&self) -> Vec<Arc<Language>> { pub fn to_vec(&self) -> Vec<Arc<Language>> {

View file

@ -315,10 +315,13 @@ impl Peer {
"incoming response: requester resumed" "incoming response: requester resumed"
); );
} else { } else {
let message_type = proto::build_typed_envelope(connection_id, incoming)
.map(|p| p.payload_type_name());
tracing::warn!( tracing::warn!(
%connection_id, %connection_id,
message_id, message_id,
responding_to, responding_to,
message_type,
"incoming response: unknown request" "incoming response: unknown request"
); );
} }

View file

@ -2751,18 +2751,20 @@ mod tests {
} }
#[gpui::test] #[gpui::test]
fn test_bundled_languages(cx: &mut AppContext) { async fn test_bundled_languages(cx: &mut TestAppContext) {
let settings = SettingsStore::test(cx); let settings = cx.update(|cx| SettingsStore::test(cx));
cx.set_global(settings); cx.set_global(settings);
let mut languages = LanguageRegistry::test(); let mut languages = LanguageRegistry::test();
languages.set_executor(cx.background_executor().clone()); languages.set_executor(cx.executor().clone());
let languages = Arc::new(languages); let languages = Arc::new(languages);
let node_runtime = node_runtime::FakeNodeRuntime::new(); 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() { 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<AppState> { fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {