diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs index 9cfd6c35af7..19aa33fe978 100644 --- a/crates/rust-analyzer/src/dispatch.rs +++ b/crates/rust-analyzer/src/dispatch.rs @@ -1,4 +1,4 @@ -//! A visitor for downcasting arbitrary request (JSON) into a specific type. +//! See [RequestDispatcher]. use std::{fmt, panic, thread}; use serde::{de::DeserializeOwned, Serialize}; @@ -10,14 +10,30 @@ use crate::{ LspError, Result, }; +/// A visitor for routing a raw JSON request to an appropriate handler function. +/// +/// Most requests are read-only and async and are handled on the threadpool +/// (`on` method). +/// +/// Some read-only requests are latency sensitive, and are immediately handled +/// on the main loop thread (`on_sync`). These are typically typing-related +/// requests. +/// +/// Some requests modify the state, and are run on the main thread to get +/// `&mut` (`on_sync_mut`). +/// +/// Read-only requests are wrapped into `catch_unwind` -- they don't modify the +/// state, so it's OK to recover from their failures. pub(crate) struct RequestDispatcher<'a> { pub(crate) req: Option, pub(crate) global_state: &'a mut GlobalState, } impl<'a> RequestDispatcher<'a> { - /// Dispatches the request onto the current thread - pub(crate) fn on_sync( + /// Dispatches the request onto the current thread, given full access to + /// mutable global state. Unlike all other methods here, this one isn't + /// guarded by `catch_unwind`, so, please, don't make bugs :-) + pub(crate) fn on_sync_mut( &mut self, f: fn(&mut GlobalState, R::Params) -> Result, ) -> Result<&mut Self> @@ -26,26 +42,40 @@ impl<'a> RequestDispatcher<'a> { R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug + 'static, R::Result: Serialize + 'static, { - let (id, params) = match self.parse::() { + let (id, params, panic_context) = match self.parse::() { Some(it) => it, None => return Ok(self), }; - let global_state = panic::AssertUnwindSafe(&mut *self.global_state); + let _pctx = stdx::panic_context::enter(panic_context); + + let result = f(&mut self.global_state, params); + let response = result_to_response::(id, result); + + self.global_state.respond(response); + Ok(self) + } + + /// Dispatches the request onto the current thread. + pub(crate) fn on_sync( + &mut self, + f: fn(GlobalStateSnapshot, R::Params) -> Result, + ) -> Result<&mut Self> + where + R: lsp_types::request::Request + 'static, + R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug + 'static, + R::Result: Serialize + 'static, + { + let (id, params, panic_context) = match self.parse::() { + Some(it) => it, + None => return Ok(self), + }; + let global_state_snapshot = self.global_state.snapshot(); let result = panic::catch_unwind(move || { - // Make sure that the whole AssertUnwindSafe is moved into the - // closure, and not just its field. - let panic::AssertUnwindSafe(global_state) = { global_state }; - - let _pctx = stdx::panic_context::enter(format!( - "\nversion: {}\nrequest: {} {:#?}", - env!("REV"), - R::METHOD, - params - )); - f(global_state, params) + let _pctx = stdx::panic_context::enter(panic_context); + f(global_state_snapshot, params) }); - let response = result_to_response::(id, result); + let response = thread_result_to_response::(id, result); self.global_state.respond(response); Ok(self) @@ -61,7 +91,7 @@ impl<'a> RequestDispatcher<'a> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug + 'static, R::Result: Serialize + 'static, { - let (id, params) = match self.parse::() { + let (id, params, panic_context) = match self.parse::() { Some(it) => it, None => return self, }; @@ -70,15 +100,10 @@ impl<'a> RequestDispatcher<'a> { let world = self.global_state.snapshot(); move || { let result = panic::catch_unwind(move || { - let _pctx = stdx::panic_context::enter(format!( - "\nversion: {}\nrequest: {} {:#?}", - env!("REV"), - R::METHOD, - params - )); + let _pctx = stdx::panic_context::enter(panic_context); f(world, params) }); - let response = result_to_response::(id, result); + let response = thread_result_to_response::(id, result); Task::Response(response) } }); @@ -98,10 +123,10 @@ impl<'a> RequestDispatcher<'a> { } } - fn parse(&mut self) -> Option<(lsp_server::RequestId, R::Params)> + fn parse(&mut self) -> Option<(lsp_server::RequestId, R::Params, String)> where R: lsp_types::request::Request + 'static, - R::Params: DeserializeOwned + 'static, + R::Params: DeserializeOwned + fmt::Debug + 'static, { let req = match &self.req { Some(req) if req.method == R::METHOD => self.req.take().unwrap(), @@ -110,7 +135,11 @@ impl<'a> RequestDispatcher<'a> { let res = crate::from_json(R::METHOD, req.params); match res { - Ok(params) => Some((req.id, params)), + Ok(params) => { + let panic_context = + format!("\nversion: {}\nrequest: {} {:#?}", env!("REV"), R::METHOD, params); + Some((req.id, params, panic_context)) + } Err(err) => { let response = lsp_server::Response::new_err( req.id, @@ -124,7 +153,7 @@ impl<'a> RequestDispatcher<'a> { } } -fn result_to_response( +fn thread_result_to_response( id: lsp_server::RequestId, result: thread::Result>, ) -> lsp_server::Response @@ -134,8 +163,37 @@ where R::Result: Serialize + 'static, { match result { - Ok(Ok(resp)) => lsp_server::Response::new_ok(id, &resp), - Ok(Err(e)) => match e.downcast::() { + Ok(result) => result_to_response::(id, result), + Err(panic) => { + let mut message = "server panicked".to_string(); + + let panic_message = panic + .downcast_ref::() + .map(String::as_str) + .or_else(|| panic.downcast_ref::<&str>().copied()); + + if let Some(panic_message) = panic_message { + message.push_str(": "); + message.push_str(panic_message) + }; + + lsp_server::Response::new_err(id, lsp_server::ErrorCode::InternalError as i32, message) + } + } +} + +fn result_to_response( + id: lsp_server::RequestId, + result: Result, +) -> lsp_server::Response +where + R: lsp_types::request::Request + 'static, + R::Params: DeserializeOwned + 'static, + R::Result: Serialize + 'static, +{ + match result { + Ok(resp) => lsp_server::Response::new_ok(id, &resp), + Err(e) => match e.downcast::() { Ok(lsp_error) => lsp_server::Response::new_err(id, lsp_error.code, lsp_error.message), Err(e) => { if is_cancelled(&*e) { @@ -153,21 +211,6 @@ where } } }, - Err(panic) => { - let mut message = "server panicked".to_string(); - - let panic_message = panic - .downcast_ref::() - .map(String::as_str) - .or_else(|| panic.downcast_ref::<&str>().copied()); - - if let Some(panic_message) = panic_message { - message.push_str(": "); - message.push_str(panic_message) - }; - - lsp_server::Response::new_err(id, lsp_server::ErrorCode::InternalError as i32, message) - } } } diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 8995d5a9b1a..eaf417117e5 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -520,24 +520,20 @@ impl GlobalState { } RequestDispatcher { req: Some(req), global_state: self } - .on_sync::(|s, ()| { + .on_sync_mut::(|s, ()| { s.fetch_workspaces_request(); s.fetch_workspaces_if_needed(); Ok(()) })? - .on_sync::(|s, p| handlers::handle_join_lines(s.snapshot(), p))? - .on_sync::(|s, p| handlers::handle_on_enter(s.snapshot(), p))? - .on_sync::(|s, ()| { + .on_sync_mut::(|s, ()| { s.shutdown_requested = true; Ok(()) })? - .on_sync::(|s, p| { - handlers::handle_selection_range(s.snapshot(), p) - })? - .on_sync::(|s, p| { - handlers::handle_matching_brace(s.snapshot(), p) - })? - .on_sync::(|s, p| handlers::handle_memory_usage(s, p))? + .on_sync_mut::(|s, p| handlers::handle_memory_usage(s, p))? + .on_sync::(handlers::handle_join_lines)? + .on_sync::(handlers::handle_on_enter)? + .on_sync::(handlers::handle_selection_range)? + .on_sync::(handlers::handle_matching_brace)? .on::(handlers::handle_analyzer_status) .on::(handlers::handle_syntax_tree) .on::(handlers::handle_view_hir)