дзеркало
https://github.com/l0ng-ai/openidea.git
синхронізовано 2026-11-09 11:15:40 +02:00
fix(lsp): Start JSON-RPC requests after the notifications before them
ide-jsonrpc handled incoming notifications in order on a dedicated thread but started each incoming request on its own blocking task right away, so a request could be handled before a notification that arrived ahead of it. The mock language server in the ide-lsp tests answered `textDocument/references` before applying the `didOpen` of a file opened for a project-wide search (missing results, a search finishing before the test could cancel it or open the file), and on the client side an ACP `session/request_permission` could overtake the `session/update` that announced its tool call (ide-agent polled for up to 200 ms to cover it). A request now starts once every notification received before it has been handled; requests still run concurrently. The ide-agent polling is gone. Tests no longer depend on timing: - the references tests hold the mock's answer with a gate and cancel or open the file while it is held, instead of sleeping and measuring; - the indexing retry ends indexing once the first answer is out; - the crash/restart test counts Ready/Crashed statuses instead of reading the last one, which a restart can already have replaced; - the mock records a notification under the same lock as its effect, and `eventually` waits up to 30 s; - the TCP tests hold their port bound (not listening) instead of freeing it for another test to take, and listen only after the command started. Claude-Session: https://claude.ai/code/session_01VUYsdXA2nkF5v6zsC93dpn
Цей коміт міститься в:
1 джерело
2fe6b7b87b
коміт
00a647daa2
6 files changed
+126
-53
No files matched your search
@@ -202,7 +202,8 @@ pub mod codes {
|
||||
///
|
||||
/// Calls run on blocking-capable threads, never on the reader task, so a slow
|
||||
/// handler cannot stall response delivery. Notifications are delivered one at
|
||||
/// a time in arrival order; requests run concurrently.
|
||||
/// a time in arrival order; requests run concurrently, each once every
|
||||
/// notification that arrived before it has been handled.
|
||||
pub trait Handler: Send + Sync + 'static {
|
||||
fn on_notification(&self, method: &str, params: Value);
|
||||
/// Respond to a server→client request (e.g. `workspace/configuration`).
|
||||
@@ -219,6 +220,13 @@ impl Handler for NoopHandler {
|
||||
}
|
||||
}
|
||||
|
||||
/// Work for the in-order notification thread.
|
||||
enum Inbound {
|
||||
Notification(String, Value),
|
||||
/// A request may start: the notifications before it are handled.
|
||||
Release(oneshot::Sender<()>),
|
||||
}
|
||||
|
||||
type Pending = HashMap<i64, oneshot::Sender<Result<Value, RpcError>>>;
|
||||
|
||||
struct Inner {
|
||||
@@ -499,13 +507,20 @@ impl Peer {
|
||||
}
|
||||
|
||||
// Notifications are delivered in order on a dedicated thread.
|
||||
let (notif_tx, notif_rx) = std::sync::mpsc::channel::<(String, Value)>();
|
||||
let (notif_tx, notif_rx) = std::sync::mpsc::channel::<Inbound>();
|
||||
{
|
||||
let handler = handler.clone();
|
||||
std::thread::Builder::new()
|
||||
.name("jsonrpc-notify".into())
|
||||
.spawn(move || {
|
||||
while let Ok((method, params)) = notif_rx.recv() {
|
||||
while let Ok(inbound) = notif_rx.recv() {
|
||||
let (method, params) = match inbound {
|
||||
Inbound::Notification(method, params) => (method, params),
|
||||
Inbound::Release(start) => {
|
||||
let _ = start.send(());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let h = handler.clone();
|
||||
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
h.on_notification(&method, params)
|
||||
@@ -680,12 +695,7 @@ fn parse_id(id: &Value) -> Option<i64> {
|
||||
}
|
||||
}
|
||||
|
||||
fn dispatch(
|
||||
inner: &Arc<Inner>,
|
||||
handler: &Arc<dyn Handler>,
|
||||
notif_tx: &std::sync::mpsc::Sender<(String, Value)>,
|
||||
body: &[u8],
|
||||
) {
|
||||
fn dispatch(inner: &Arc<Inner>, handler: &Arc<dyn Handler>, notif_tx: &std::sync::mpsc::Sender<Inbound>, body: &[u8]) {
|
||||
tracing::trace!(target: "ide_jsonrpc::wire", "<-- {}", String::from_utf8_lossy(body));
|
||||
let msg: Value = match serde_json::from_slice(body) {
|
||||
Ok(v) => v,
|
||||
@@ -706,7 +716,11 @@ fn dispatch(
|
||||
Some(id) if !id.is_null() => {
|
||||
let handler = handler.clone();
|
||||
let inner = inner.clone();
|
||||
let (start, started) = oneshot::channel();
|
||||
let _ = notif_tx.send(Inbound::Release(start));
|
||||
tokio::spawn(async move {
|
||||
// E.g. a `didOpen` before a request must be applied when the request is answered.
|
||||
let _ = started.await;
|
||||
let m = method.clone();
|
||||
let result = tokio::task::spawn_blocking(move || handler.on_request(&m, params)).await;
|
||||
let reply = match result {
|
||||
@@ -723,7 +737,7 @@ fn dispatch(
|
||||
});
|
||||
}
|
||||
_ => {
|
||||
let _ = notif_tx.send((method, params));
|
||||
let _ = notif_tx.send(Inbound::Notification(method, params));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -251,6 +251,27 @@ async fn server_to_client_request() {
|
||||
assert!(matches!(err, RpcError::Server { code: codes::METHOD_NOT_FOUND, .. }));
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn requests_start_after_the_notifications_before_them() {
|
||||
// `seen` answers with the notifications handled so far; handling one is slow.
|
||||
struct Server(Recorder);
|
||||
impl Handler for Server {
|
||||
fn on_notification(&self, method: &str, params: Value) {
|
||||
std::thread::sleep(Duration::from_millis(20));
|
||||
self.0.on_notification(method, params)
|
||||
}
|
||||
fn on_request(&self, _: &str, _: Value) -> Result<Value, (i64, String)> {
|
||||
Ok(json!(self.0.notifications.lock().len()))
|
||||
}
|
||||
}
|
||||
let (client, _server) = pair(Arc::new(NoopHandler), Arc::new(Server(Recorder::default())));
|
||||
for n in 1..=5 {
|
||||
client.notify("open", n).unwrap();
|
||||
let seen: usize = client.request("seen", (), None).await.unwrap();
|
||||
assert_eq!(seen, n);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn slow_handler_does_not_block_responses() {
|
||||
// The client handles a slow server→client request while its own request completes.
|
||||
|
||||
Reference in new issue
Заблокувати користувача