10080: internal: don't shut up the compiler when it says the code's buggy r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2021-08-30 07:58:59 +00:00 committed by GitHub
commit ceecf853ee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 96 additions and 57 deletions

View file

@ -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<lsp_server::Request>,
pub(crate) global_state: &'a mut GlobalState,
}
impl<'a> RequestDispatcher<'a> {
/// Dispatches the request onto the current thread
pub(crate) fn on_sync<R>(
/// 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<R>(
&mut self,
f: fn(&mut GlobalState, R::Params) -> Result<R::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::<R>() {
let (id, params, panic_context) = match self.parse::<R>() {
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::<R>(id, result);
self.global_state.respond(response);
Ok(self)
}
/// Dispatches the request onto the current thread.
pub(crate) fn on_sync<R>(
&mut self,
f: fn(GlobalStateSnapshot, R::Params) -> Result<R::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::<R>() {
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::<R>(id, result);
let response = thread_result_to_response::<R>(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::<R>() {
let (id, params, panic_context) = match self.parse::<R>() {
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::<R>(id, result);
let response = thread_result_to_response::<R>(id, result);
Task::Response(response)
}
});
@ -98,10 +123,10 @@ impl<'a> RequestDispatcher<'a> {
}
}
fn parse<R>(&mut self) -> Option<(lsp_server::RequestId, R::Params)>
fn parse<R>(&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<R>(
fn thread_result_to_response<R>(
id: lsp_server::RequestId,
result: thread::Result<Result<R::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::<LspError>() {
Ok(result) => result_to_response::<R>(id, result),
Err(panic) => {
let mut message = "server panicked".to_string();
let panic_message = panic
.downcast_ref::<String>()
.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<R>(
id: lsp_server::RequestId,
result: Result<R::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::<LspError>() {
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::<String>()
.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)
}
}
}

View file

@ -520,24 +520,20 @@ impl GlobalState {
}
RequestDispatcher { req: Some(req), global_state: self }
.on_sync::<lsp_ext::ReloadWorkspace>(|s, ()| {
.on_sync_mut::<lsp_ext::ReloadWorkspace>(|s, ()| {
s.fetch_workspaces_request();
s.fetch_workspaces_if_needed();
Ok(())
})?
.on_sync::<lsp_ext::JoinLines>(|s, p| handlers::handle_join_lines(s.snapshot(), p))?
.on_sync::<lsp_ext::OnEnter>(|s, p| handlers::handle_on_enter(s.snapshot(), p))?
.on_sync::<lsp_types::request::Shutdown>(|s, ()| {
.on_sync_mut::<lsp_types::request::Shutdown>(|s, ()| {
s.shutdown_requested = true;
Ok(())
})?
.on_sync::<lsp_types::request::SelectionRangeRequest>(|s, p| {
handlers::handle_selection_range(s.snapshot(), p)
})?
.on_sync::<lsp_ext::MatchingBrace>(|s, p| {
handlers::handle_matching_brace(s.snapshot(), p)
})?
.on_sync::<lsp_ext::MemoryUsage>(|s, p| handlers::handle_memory_usage(s, p))?
.on_sync_mut::<lsp_ext::MemoryUsage>(|s, p| handlers::handle_memory_usage(s, p))?
.on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)?
.on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)?
.on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)?
.on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)?
.on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)
.on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree)
.on::<lsp_ext::ViewHir>(handlers::handle_view_hir)