From c6709ffe05e0e8bec5f75a0e0fc0c469ff8cb7c0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 1 Jul 2024 14:30:21 +0200 Subject: [PATCH] Improve error message when the proc-macro server unexpectedly exits --- crates/proc-macro-api/src/lib.rs | 4 ++ crates/proc-macro-api/src/process.rs | 80 ++++++++++++++++++---------- crates/proc-macro-srv/src/lib.rs | 2 + crates/rust-analyzer/src/reload.rs | 30 ++++++++--- 4 files changed, 79 insertions(+), 37 deletions(-) diff --git a/crates/proc-macro-api/src/lib.rs b/crates/proc-macro-api/src/lib.rs index 4c65dd5969cf..3a915e668bbf 100644 --- a/crates/proc-macro-api/src/lib.rs +++ b/crates/proc-macro-api/src/lib.rs @@ -130,6 +130,10 @@ impl ProcMacroServer { Err(message) => Err(ServerError { message, io: None }), } } + + pub fn exited(&self) -> Option<&ServerError> { + self.process.exited() + } } impl ProcMacro { diff --git a/crates/proc-macro-api/src/process.rs b/crates/proc-macro-api/src/process.rs index 2b1a791827a7..c965257a5c52 100644 --- a/crates/proc-macro-api/src/process.rs +++ b/crates/proc-macro-api/src/process.rs @@ -2,8 +2,9 @@ use std::{ io::{self, BufRead, BufReader, Read, Write}, + panic::AssertUnwindSafe, process::{Child, ChildStdin, ChildStdout, Command, Stdio}, - sync::{Arc, Mutex}, + sync::{Arc, Mutex, OnceLock}, }; use paths::AbsPath; @@ -22,6 +23,8 @@ pub(crate) struct ProcMacroProcessSrv { state: Mutex, version: u32, mode: SpanMode, + /// Populated when the server exits. + exited: OnceLock>, } #[derive(Debug)] @@ -29,8 +32,6 @@ struct ProcessSrvState { process: Process, stdin: ChildStdin, stdout: BufReader, - /// Populated when the server exits. - server_exited: Option, } impl ProcMacroProcessSrv { @@ -44,9 +45,10 @@ impl ProcMacroProcessSrv { let (stdin, stdout) = process.stdio().expect("couldn't access child stdio"); io::Result::Ok(ProcMacroProcessSrv { - state: Mutex::new(ProcessSrvState { process, stdin, stdout, server_exited: None }), + state: Mutex::new(ProcessSrvState { process, stdin, stdout }), version: 0, mode: SpanMode::Id, + exited: OnceLock::new(), }) }; let mut srv = create_srv(true)?; @@ -77,6 +79,10 @@ impl ProcMacroProcessSrv { } } + pub(crate) fn exited(&self) -> Option<&ServerError> { + self.exited.get().map(|it| &it.0) + } + pub(crate) fn version(&self) -> u32 { self.version } @@ -118,36 +124,52 @@ impl ProcMacroProcessSrv { } pub(crate) fn send_task(&self, req: Request) -> Result { - let state = &mut *self.state.lock().unwrap(); - if let Some(server_error) = &state.server_exited { - return Err(server_error.clone()); + if let Some(server_error) = self.exited.get() { + return Err(server_error.0.clone()); } + let state = &mut *self.state.lock().unwrap(); let mut buf = String::new(); - send_request(&mut state.stdin, &mut state.stdout, req, &mut buf).map_err(|e| { - if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) { - match state.process.child.try_wait() { - Ok(None) => e, - Ok(Some(status)) => { - let mut msg = String::new(); - if !status.success() { - if let Some(stderr) = state.process.child.stderr.as_mut() { - _ = stderr.read_to_string(&mut msg); + send_request(&mut state.stdin, &mut state.stdout, req, &mut buf) + .and_then(|res| { + res.ok_or_else(|| { + let message = "proc-macro server did not respond with data".to_owned(); + ServerError { + io: Some(Arc::new(io::Error::new( + io::ErrorKind::BrokenPipe, + message.clone(), + ))), + message, + } + }) + }) + .map_err(|e| { + if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) { + match state.process.child.try_wait() { + Ok(None) | Err(_) => e, + Ok(Some(status)) => { + let mut msg = String::new(); + if !status.success() { + if let Some(stderr) = state.process.child.stderr.as_mut() { + _ = stderr.read_to_string(&mut msg); + } } + let server_error = ServerError { + message: format!( + "proc-macro server exited with {status}{}{msg}", + if msg.is_empty() { "" } else { ": " } + ), + io: None, + }; + // `AssertUnwindSafe` is fine here, we already correct initialized + // server_error at this point. + self.exited.get_or_init(|| AssertUnwindSafe(server_error)).0.clone() } - let server_error = ServerError { - message: format!("server exited with {status}: {msg}"), - io: None, - }; - state.server_exited = Some(server_error.clone()); - server_error } - Err(_) => e, + } else { + e } - } else { - e - } - }) + }) } } @@ -201,7 +223,7 @@ fn send_request( mut reader: &mut impl BufRead, req: Request, buf: &mut String, -) -> Result { +) -> Result, ServerError> { req.write(write_json, &mut writer).map_err(|err| ServerError { message: "failed to write request".into(), io: Some(Arc::new(err)), @@ -210,5 +232,5 @@ fn send_request( message: "failed to read response".into(), io: Some(Arc::new(err)), })?; - res.ok_or_else(|| ServerError { message: "server exited".into(), io: None }) + Ok(res) } diff --git a/crates/proc-macro-srv/src/lib.rs b/crates/proc-macro-srv/src/lib.rs index 34851ee0bee8..e6281035e1a2 100644 --- a/crates/proc-macro-srv/src/lib.rs +++ b/crates/proc-macro-srv/src/lib.rs @@ -204,6 +204,8 @@ fn expand_ra_span( let macro_body = macro_body.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table); let attributes = attributes.map(|it| it.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table)); + // Note, we spawn a new thread here so that thread locals allocation don't accumulate (this + // includes the proc-macro symbol interner) let result = thread::scope(|s| { let thread = thread::Builder::new() .stack_size(EXPANDER_STACK_SIZE) diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 7b9a10fdb8c0..1039daf850ca 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -184,14 +184,28 @@ impl GlobalState { message.push_str(err); message.push_str("\n\n"); }; - if let Some(Err(err)) = proc_macro_client { - status.health |= lsp_ext::Health::Warning; - format_to!( - message, - "Failed spawning proc-macro server for workspace `{}`: {err}", - ws.manifest_or_root() - ); - message.push_str("\n\n"); + match proc_macro_client { + Some(Err(err)) => { + status.health |= lsp_ext::Health::Warning; + format_to!( + message, + "Failed spawning proc-macro server for workspace `{}`: {err}", + ws.manifest_or_root() + ); + message.push_str("\n\n"); + } + Some(Ok(client)) => { + if let Some(err) = client.exited() { + status.health |= lsp_ext::Health::Warning; + format_to!( + message, + "proc-macro server for workspace `{}` exited: {err}", + ws.manifest_or_root() + ); + message.push_str("\n\n"); + } + } + _ => (), } } }