Skip to content

Commit

Permalink
lsp: Process LSP messages sequentially
Browse files Browse the repository at this point in the history
As @ConradIrwin found out in #23301 the LSP messages could be processed out of order after #23122; this PR makes our
approach to event processing a bit more conservative - instead of detaching the parsing task we'll now await them in the main loop.
This keeps the benefit of #23122 in that deserialization still happens off the main thread, but some of the performance benefit of #23122 could've been a result of the buggy interaction
that Conrad discovered.
  • Loading branch information
osiewicz committed Jan 17, 2025
1 parent cf7e541 commit 1d360b0
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions crates/lsp/src/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ pub use lsp_types::*;

use anyhow::{anyhow, Context, Result};
use collections::HashMap;
use futures::{channel::oneshot, io::BufWriter, select, AsyncRead, AsyncWrite, Future, FutureExt};
use futures::{
channel::oneshot, future::BoxFuture, io::BufWriter, select, AsyncRead, AsyncWrite, Future,
FutureExt,
};
use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, SharedString, Task};
use parking_lot::{Mutex, RwLock};
use postage::{barrier, prelude::Stream};
Expand Down Expand Up @@ -45,7 +48,8 @@ const CONTENT_LEN_HEADER: &str = "Content-Length: ";
const LSP_REQUEST_TIMEOUT: Duration = Duration::from_secs(60 * 2);
const SERVER_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5);

type NotificationHandler = Arc<dyn Send + Sync + Fn(Option<RequestId>, Value, AsyncAppContext)>;
type NotificationHandler =
Arc<dyn Send + Sync + Fn(Option<RequestId>, Value, AsyncAppContext) -> BoxFuture<'static, ()>>;
type ResponseHandler = Box<dyn Send + FnOnce(Result<String, Error>)>;
type IoHandler = Box<dyn Send + FnMut(IoKind, &str)>;

Expand Down Expand Up @@ -530,7 +534,7 @@ impl LanguageServer {
{
let mut notification_handlers = notification_handlers.lock();
if let Some(handler) = notification_handlers.get_mut(msg.method.as_str()) {
handler(msg.id, msg.params.unwrap_or(Value::Null), cx.clone());
handler(msg.id, msg.params.unwrap_or(Value::Null), cx.clone()).await;
} else {
drop(notification_handlers);
on_unhandled_notification(msg);
Expand Down Expand Up @@ -959,7 +963,7 @@ impl LanguageServer {
callback(params, cx);
}
})
.detach();
.boxed()
}),
);
assert!(
Expand Down Expand Up @@ -1040,7 +1044,9 @@ impl LanguageServer {
}
}
})
.detach();
.boxed()
} else {
async {}.boxed()
}
}),
);
Expand Down

0 comments on commit 1d360b0

Please sign in to comment.